From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yskoh@mellanox.com>
Received: from EUR01-HE1-obe.outbound.protection.outlook.com
(mail-he1eur01on0054.outbound.protection.outlook.com [104.47.0.54])
by dpdk.org (Postfix) with ESMTP id EAAAE1B6E0;
Fri, 10 Nov 2017 22:42:47 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;
bh=ZOosQaOPscoL877ag9d6RojdfBLGCQuLf4cxVnmvz2I=;
b=vlT2Z4VqB/DVhSeNRX6GZbfw3WBV2qOWHb05mJcrkEoUb+CX5RCM9Wx9pbFSbcFo9EPvN0OVan8NPB2EzCGcnXmq+PE29pIrLglJRMeKWDNIUvxQQcLfPC90Ia50y+jsyCYodNAbm5XNOWNCu4zLsmGgu0BEml/poahnmkFBrmo=
Authentication-Results: spf=none (sender IP is )
smtp.mailfrom=yskoh@mellanox.com;
Received: from yongseok-MBP.local (209.116.155.178) by
HE1PR0501MB2044.eurprd05.prod.outlook.com (2603:10a6:3:35::22) with Microsoft
SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.197.13; Fri, 10
Nov 2017 21:42:43 +0000
Date: Fri, 10 Nov 2017 13:42:32 -0800
From: Yongseok Koh <yskoh@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Ori Kam <orika@mellanox.com>, nelio.laranjeiro@6wind.com, dev@dpdk.org,
stable@dpdk.org
Message-ID: <20171110214231.GB4189@yongseok-MBP.local>
References: <1510243472-24872-1-git-send-email-orika@mellanox.com>
<20171109223029.GA3599@yongseok-MBP.local>
<20171110102206.GG24849@6wind.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171110102206.GG24849@6wind.com>
User-Agent: Mutt/1.7.2 (2016-11-26)
X-Originating-IP: [209.116.155.178]
X-ClientProxiedBy: SN4PR0801CA0007.namprd08.prod.outlook.com
(2603:10b6:803:29::17) To HE1PR0501MB2044.eurprd05.prod.outlook.com
(2603:10a6:3:35::22)
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 2c0e96ff-1dda-45d7-4a11-08d52883fa69
X-MS-Office365-Filtering-HT: Tenant
X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0;
RULEID:(22001)(48565401081)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(2017052603199);
SRVR:HE1PR0501MB2044;
X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2044;
3:X+YHD+vxW0Br4X424vZ9oWqhdRKhrqouA3miErfZrALiUiWdR8/NhoYKOt+RbJ3Csgz1gq2APyoQxiCgAlpFTR+m6nzWZKS02F2NEiLR2bT7JN6k5yo0KBMP9r/AxfzoQBlFMkdmy7sLOhYS9RjaLqggs6zmsKrhqWgx/IH366zkrNTa89PoKFp63EKoX/JXZcErYec00oxM2IhoColSWDe+M/QwN+PXhkD+Hwz7Hv2hYW1y8/mFveAHwXeuaTlc;
25:sI8/Puv1uKToFXeDTcABncLqS7/GvTpEAKGNyb1WdK0dM2oz4FXbQ194ql8LQgtMYiW8o+CxvVNmE5rfJPR99DuBTQXTyEGeWayHe6PwyAno7CiVGX1EJd09G238M1eRvxUUd1ouovrkknfDp2/4F6CCcoPgSRDIfMTCDgXBYECDJUx83Fuouh0h4PJ/yQnRKYwYv/O6Hi/vYfXgOiH5TnyWYfIuSxf+vYgB+M4gfZx40d1nt1wjl2+21roJ9ONhSnsbmUdde89kPMbqu3SyXuAPB7DCch6LWU/MFy05CEjd2s7mFlJG5xKrycIMiCf/SmecW2OqnCR/wt4vL/FHHA==;
31:CSaHwhfTUvApiehBDoTEPbvxkdWnGXYqDqebsoQ2t5ZDcPav0AVaIexDe6GbXSa9llzYZUxV/uzVVHaOQChN9Ffw1mY1XZzHrQS+2zsJrm2NH7pMogWA8TVrN4YYWNnYEsTkL9Jq5Cr4GWJbEpO6W6kSVorRCxCfZAauCtzK0d/GEYG59p0Od4XOHiq/yu0bIYe+eR6e2Ztqo4sOxIMa5E9RlZD7oAGHBo+BuZACAJ4=
X-MS-TrafficTypeDiagnostic: HE1PR0501MB2044:
X-LD-Processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2044;
20:biuS47yFAS/L6RPiLLGXuWn9rM3DyegaGABr0vk5emtjM6bSjFj11R6l5o5CZNfsLmb+Z+XF7gZ89VIVQ6VIVQhesN+w4D3364R2WodM2pfZdBnvv11vsnue3raqbqRoKqv/7cHyBiRE9fqqmauWr6oR0JzHP4hnHRih61lGKTXfQ0CIKeA8p1uoz5O0jbMeu6eTpOcCGvm3g5z12x2XTiKsc+e4W9A0by5IyVZHkL+T/ElVKHcfKrE3UPTdOYiSz1b+3uQqKbhikUwcu1SHyrh2FKaJS/MAP+BMaCga49yBKCkVCUfRs60cFR3Q64CrYcuUPhW8Oa8vGikXLJKIZk/35rBPGSQhJ+fqq0x1KUtZUVCD+XbNSLQCHktYtwMuCAi+hEvfpAotbsfHb3w6m3evO10/+B132CTJzcKOsYIpDLeFssayIMb9loVn6zERbtWYx5MhiShcLtfdpZ0U48o1EyAAjn+4aEk5Wu9oqsBEuw6cDNGb48oWxVe9xqjD;
4:GB7aYicmpdyir2RBdfzmcENcPKK+AtsG7D6igpJfUj9DzYafRlUz18sCb11AQmSn9qUB59S2hq/hxG3fm3K5mKUeb0hOvfjQPIgJfptgW+BIoAHuLU88CVEN2GALcqRLg1u2nrFMa0V7Jxe1VTHhOTLkpgBjuoKaSgwJ0QdBOnw0+Cicb82SFQhXPTjSfxkEA89MlcdbadakDQ+KZmom9jVA4Gx0f5h1sxm69nbNT/qHVgscSfwR1qNzTIn3Ex7e8fBmx4VGFBJ5YYK8Z7lXAw==
X-Exchange-Antispam-Report-Test: UriScan:;
X-Microsoft-Antispam-PRVS: <HE1PR0501MB2044BCE33E77A8A4A70B34A8C3540@HE1PR0501MB2044.eurprd05.prod.outlook.com>
X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0;
RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(10201501046)(3231022)(93006095)(93001095)(100000703101)(100105400095)(6055026)(6041248)(20161123558100)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123560025)(20161123555025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);
SRVR:HE1PR0501MB2044; BCL:0; PCL:0;
RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);
SRVR:HE1PR0501MB2044;
X-Forefront-PRVS: 0487C0DB7E
X-Forefront-Antispam-Report: SFV:NSPM;
SFS:(10009020)(6009001)(346002)(376002)(39860400002)(199003)(189002)(24454002)(86362001)(83506002)(189998001)(16586007)(58126008)(316002)(25786009)(68736007)(16526018)(5660300001)(50466002)(6666003)(2950100002)(4326008)(6916009)(6116002)(229853002)(478600001)(105586002)(2906002)(1076002)(76176999)(23726003)(6246003)(106356001)(54356999)(50986999)(6506006)(101416001)(3846002)(55016002)(98436002)(53936002)(81156014)(97736004)(8676002)(81166006)(9686003)(7736002)(33656002)(66066001)(305945005)(8936002)(47776003)(18370500001);
DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0501MB2044; H:yongseok-MBP.local; FPR:;
SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en;
Received-SPF: None (protection.outlook.com: mellanox.com does not designate
permitted sender hosts)
X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; HE1PR0501MB2044;
23:9LHwwkCRpUvDNaHo8OrwAdyJxRsZYL/fzaG4OtF?=
=?us-ascii?Q?bVuIeU1uEQt/tL6mwV8/q7ociHB0bGBRzmbqwDofO06jRXYzpkaf21orA7lt?=
=?us-ascii?Q?sVb64GJ42jphjUwc0J9eDQlVFD1TntlJxb28LCIjVoG4tkDUKlwSJ3q4zA/a?=
=?us-ascii?Q?+L6Ha5PYeh2kP30jzeBJUWMi2QZlDl8JSNxGt9h4Es19HaG5GAzyKldpRc+o?=
=?us-ascii?Q?AM1ZbkXojiKDAQK2tvpxaKh3bMga2DI07MTn2FmSLuh9BDc/Vrb0742LbHJc?=
=?us-ascii?Q?S8mvknnS16KwNOj0yRIdubLNTX6AbsJXcNtNoro0kdSYvC5jMloiIfHtbrXt?=
=?us-ascii?Q?9r4X5zc9c1+p1uR/gNoLozGPZFXeFH8oLUcTcqZXT7ofVAgXgQJ5oJD8EsDK?=
=?us-ascii?Q?0i9IznW2UxFX29ENKEvJfc+eYF1N8LlI0HSQSBeoNfmRU+lTu154cmT2Do0v?=
=?us-ascii?Q?l36uIWTM0xDt1eJ2V2I0cC5ArXQxaUG22us+1QbbhwNuV0O8MGC4RZjdAXq0?=
=?us-ascii?Q?OGLY0z9mB82UVVg25jGLOdegsFvZp5itk6bXpCDuhlDfjXV2SZZ2rOkIvhTh?=
=?us-ascii?Q?rHJeytS5KrSHkdPJCaCppowIqwS8rFmxId26QIW+PsJ61mDkqGxrnqKilvWN?=
=?us-ascii?Q?m8oQvP9qqvQkkL7YTWAxR4mdECZpg7f6BxkTV/Dtbw9pp5tsf6GrgC1VnV1B?=
=?us-ascii?Q?N60r6e1iHG/1pALE6lahRpWj+IE27Fg9sJ45RAK1oaxZ3gASQVTgpFj1yFKF?=
=?us-ascii?Q?FkrLo3vP3vAU7malmnWSvXhjC61XmczKg6otCMT70GTKkkz4KG/qL0VRh5vR?=
=?us-ascii?Q?VUzEdY+qoDlmQjSer8V7/9gSLzfhEPZrZeE7VZt2jltmPS4TJUOVCzXR7Ud5?=
=?us-ascii?Q?Voqts354AUCMV4W1sTSzzB/QOVuYl2P2zjvX+q6EidyBBI/sjlCTZN80LO/w?=
=?us-ascii?Q?9ZsESeStw6J+J3PRJaPINWT9SkP89x4TMeYS7yTX6a0e4sP4FqvCqfcPy8gO?=
=?us-ascii?Q?5WEUeCrzmO9ZL0Nni/cxVcUSonkHJg2u9iuTQvEt4wf/8guV64HOSGVrrHqy?=
=?us-ascii?Q?r9wDwcFdYGmmIL7veRbrrAcEqf5ztJVD+xD5Wrjvw4PRdGCUSunn5JYDKjS2?=
=?us-ascii?Q?EQhPAb3Pcig6P/LG/UG+i271m0Zz54kDcNTpQso5N/9n1/+yDGvtkjc1ugcp?=
=?us-ascii?Q?0lT36LGez4oW8xcGyqzqp2pHcnr2krr951fAC?=
X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2044;
6:hWyUGwhXklMEVNCSqFxMDSyTA1wDfnIzdu10NoTDu553HghoTRkYVw2A/QUNwhJ20twPQhIze7QD2vLp8eDZX9a+xJRiaRzA9RSG6U3OieJigk6a33F2dpkE4W5MhfQyCk3jp9l6XKl3ZihhWh3ozUL/39iZd+2LaLaWB3xKCB1Vj/mrhifRN8H3IP23hueVPzQO2XKXBLjhVM+XZho6zJs0t5YM9dRtpuZEGB9NAZ3QKoZ4jLJ7Q2Gt7Hq4fBWt+8zEZ265mqnpbDIjNnuWs1OspYXE6dXltXX/ABihdv6xU4pVAn0XQO5Qe7i1Lj/Tqg/UzRMeBkRC/Odwvnj+7yfGIUYDSyiZhxpNQJdB7So=;
5:c6AMVG2r9AMALnDdtT5P++9Y3HnG6MuRozGxILDaJvURYu65SYcHslTV0egSec4/XKEOVUwI8x6/3n17RR9AzY8YjXg98UjDvPp/oOp3RyWBurjjxEkLa0P2diIYGxAdv9zKisBmph/0t5xKq/ShwT31nrB31glPDPR2oD4sogk=;
24:dXYgWzSlW69Ya3nJBjp7oyPfs/I/6FG6ZrRV+qvQJ5HSz8lu+kAQ6nNEYtWeAbOLfgu3Lk6mxnP0SKJPrS+O3qWqvnUqoqs5vrIuAm5+oNs=;
7:PAl/nDbxQp8FhiGjdXUTY2Ig48ZnPmm9YQF6IH8omlsPVJJh8LQ3mXHkB27JcDrWiEB9Zmo9aUngIO2oiG8zpShkvbnqsB1h+eq6Rtrx/A1KRBh4SeF0Cxvk81aW8I9b/OaM9AMnZO2R4pAxIl2KuAfCCiPgCUWnqF7cUPvYgrbOSwlhoSYLY/ZDY8eFnDEJK4rls4xa/tOlkoNoQsnh4R2HFdvzFBIQP1/u/u13aW3Az1fKxo5LNWKqyoOjqR8a
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2017 21:42:43.7011 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 2c0e96ff-1dda-45d7-4a11-08d52883fa69
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0501MB2044
Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix number of segment
calculation
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 10 Nov 2017 21:42:48 -0000
On Fri, Nov 10, 2017 at 11:22:06AM +0100, Adrien Mazarguil wrote:
> Hi Yongseok,
>
> On Thu, Nov 09, 2017 at 02:30:30PM -0800, Yongseok Koh wrote:
> > On Thu, Nov 09, 2017 at 06:04:32PM +0200, Ori Kam wrote:
> > > The CRC size should be taken into consideration when computing
> > > the number of mbuf segments for packet on the receive path.
> > > Large packets can be dropped due to extra CRC length.
> > >
> > > Fixes: a1366b1a2be3 ("net/mlx5: add reference counter on DPDK Rx queues")
> > > Cc: stable@dpdk.org
> > > Cc: nelio.laranjeiro@6wind.com
> > >
> > > Signed-off-by: Ori Kam <orika@mellanox.com>
> > > ---
> > > drivers/net/mlx5/mlx5_rxq.c | 7 +++++--
> > > 1 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> > > index 6b29aae..701925b 100644
> > > --- a/drivers/net/mlx5/mlx5_rxq.c
> > > +++ b/drivers/net/mlx5/mlx5_rxq.c
> > > @@ -887,6 +887,8 @@ struct mlx5_rxq_ctrl*
> > > const uint16_t desc_n =
> > > desc + priv->rx_vec_en * MLX5_VPMD_DESCS_PER_LOOP;
> > > unsigned int mb_len = rte_pktmbuf_data_room_size(mp);
> > > + uint8_t crc_size =
> > > + !!(dev->data->dev_conf.rxmode.hw_strip_crc == 0) << 2;
> >
> > How about making it more explicit with ETHER_CRC_LEN? E.g.
> > uint8_t crc_size = ETHER_CRC_LEN *
> > (dev->data->dev_conf.rxmode.hw_strip_crc == 0);
> >
> > >
> > > tmpl = rte_calloc_socket("RXQ", 1,
> > > sizeof(*tmpl) +
> > > @@ -900,12 +902,13 @@ struct mlx5_rxq_ctrl*
> > > /* Enable scattered packets support for this queue if necessary. */
> > > assert(mb_len >= RTE_PKTMBUF_HEADROOM);
> >
> > You might want to make the same change for this assert?
> >
> > > if (dev->data->dev_conf.rxmode.max_rx_pkt_len <=
> > > - (mb_len - RTE_PKTMBUF_HEADROOM)) {
> > > + (mb_len - RTE_PKTMBUF_HEADROOM - crc_size)) {
> > > tmpl->rxq.sges_n = 0;
> > > } else if (dev->data->dev_conf.rxmode.enable_scatter) {
> > > unsigned int size =
> > > RTE_PKTMBUF_HEADROOM +
> > > - dev->data->dev_conf.rxmode.max_rx_pkt_len;
> > > + dev->data->dev_conf.rxmode.max_rx_pkt_len +
> > > + crc_size;
> >
> > I think there's another bugs we didn't know. If scatter is required,
> > RTE_PKTMBUF_HEADROOM is also reserved per every chained mbufs. So, it looks like
> > mb_len should be "rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM" when it
> > is declared in the beginning. Make sense?
>
> RTE_PKTMBUF_HEADROOM is actually only reserved on the first segment,
> i.e. once per mbuf chain, it should be fine.
Right, I got confused with Tx. mlx5's Rx overwrites m->data_offset when it
injects mbufs for extra segments.
> > > /*
> > > * Determine the number of SGEs needed for a full packet
> > > * and round it to the next power of two.
> > > */
> > > sges_n = log2above((size / mb_len) + !!(size % mb_len));
> > > tmpl->rxq.sges_n = sges_n;
> >
> > rxq.sges_n is 2bits, which means the max value is 3. So, if sges_n is larger
> > than 3, it would just take the last 2bits and it will result in false error
> > below. As we can't use sizeof() for bit-fields, this should be changed like:
>
> The name is perhaps confusing, sges_n is documented as a log 2 value, 1 << 3
> means 8 segments at most. Assuming default mbuf size, this allows up to
> 17280 bytes per packet excluding headroom.
>
> You're right exceeding 3 will remove the extra bits and since sizeof() can't
> be used, that's precisely the reason for the subsequent check, which makes
> sure the stored value is enough for a max_rx_pkt_len-sized packet after
> converting it back to a number of bytes.
The name wasn't confusing, I wanted to make it clearer as I thought it could
have some false negatives. But, I misread something. The sanity check can
correctly filter those cases. No bug here!
Thanks,
Yongseok