From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 To: Adrien Mazarguil Cc: Ori Kam , 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > > > --- > > > 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