From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f49.google.com (mail-vk0-f49.google.com [209.85.213.49]) by dpdk.org (Postfix) with ESMTP id 5F8E72BC3 for ; Wed, 20 Apr 2016 14:22:58 +0200 (CEST) Received: by mail-vk0-f49.google.com with SMTP id e185so56389605vkb.1 for ; Wed, 20 Apr 2016 05:22:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=infinite-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=xL7zsG7mLDz/ZVftcOD8L2vgf5EnNAq+kauQOCgQ7aw=; b=fKirkf3DTj8tlSWSPKy/4RU5t2Yz1oXOQ0fZsqjPYuF/qQT2Dt1JsWFKZbIbCRCFol 20EPKFSAxQbb/eYdQELckEmc3TjS44obAS3VxOn4jEKRe8vJ53hi2QUQ2dnKMCu5SA8Q ZN9ZI3aixhZk1S+u6zIACvWDnSPDRNdD8ej9jRfPLgEgm5cRwxm9QNafpL+EXBnbOMSZ wQVMh2N2A0r7sVz4x/z0TtlSPgo3nh+6FxqEMV7lFWjOulY5xCtHKM/F18zD6VlKYlAJ xe68WuN7Fz2j0S8yi3zcHZL1WsfCaS9HtqGNQPIsSShLTxxYYCjUEnf3HQTLMDPKQrNq Aylg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=xL7zsG7mLDz/ZVftcOD8L2vgf5EnNAq+kauQOCgQ7aw=; b=RJMfViqC5vSg5xhOeaXpkdSbT/9lWh88a20qHTSZU0kGoQFMauEheE/kJVrXJ/ZPHW X3nI13JLw5ooJdUoSCsnYtjIuiWx2CVuogrI4kWyYzPGYzqu6bmbpWKsaI0ZcjWwdvSy uIdWxxRr9fkgYaz4y3loEblNJun16bXCGhz9sOUitPZUKV1rBkB/BtcslfD6etgLw0vh nH5EZzWjeGQwIw2ZcgWdi0r01HU2VNeiZFXsctI3WQMMhpccFvqDvyCOH9B8UntCKLWY NZzFgPOziJQgoMEFJF7hwA7XEtkJrdLJN+t/oGdGg53Y8X1xe1j5ba2YhwK7MeSNZqZU kMKQ== X-Gm-Message-State: AOPr4FWhsNTFb20GHJbBdy8nx6E6WggJhBd12mHYP+GUE21aqszSDHW69VbAma9LZ9a3gyxR+vnYxAxjCePH3w== MIME-Version: 1.0 X-Received: by 10.31.194.10 with SMTP id s10mr4664779vkf.72.1461154977854; Wed, 20 Apr 2016 05:22:57 -0700 (PDT) Received: by 10.103.86.14 with HTTP; Wed, 20 Apr 2016 05:22:57 -0700 (PDT) In-Reply-To: <20160420091046.GA4080@bricha3-MOBL3> References: <20160420091046.GA4080@bricha3-MOBL3> Date: Wed, 20 Apr 2016 07:22:57 -0500 Message-ID: From: Jay Rolette To: Bruce Richardson Cc: DPDK Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] Couple of PMD questions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Apr 2016 12:22:58 -0000 On Wed, Apr 20, 2016 at 4:10 AM, Bruce Richardson < bruce.richardson@intel.com> wrote: > On Tue, Apr 19, 2016 at 03:16:37PM -0500, Jay Rolette wrote: > > In ixgbe_dev_rx_init(), there is this bit of code: > > > > /* > > * Configure the RX buffer size in the BSIZEPACKET field of > > * the SRRCTL register of the queue. > > * The value is in 1 KB resolution. Valid values can be from > > * 1 KB to 16 KB. > > */ > > buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) - > > RTE_PKTMBUF_HEADROOM); > > srrctl |= ((buf_size >> IXGBE_SRRCTL_BSIZEPKT_SHIFT) & > > IXGBE_SRRCTL_BSIZEPKT_MASK); > > > > IXGBE_WRITE_REG(hw, IXGBE_SRRCTL(rxq->reg_idx), srrctl); > > > > buf_size = (uint16_t) ((srrctl & IXGBE_SRRCTL_BSIZEPKT_MASK) << > > IXGBE_SRRCTL_BSIZEPKT_SHIFT); > > > > /* It adds dual VLAN length for supporting dual VLAN */ > > if (dev->data->dev_conf.rxmode.max_rx_pkt_len + > > 2 * IXGBE_VLAN_TAG_SIZE > buf_size) > > dev->data->scattered_rx = 1; > > > > > > Couple of questions I have about it: > > > > 1) If the caller configured the MBUF memory pool data room size to be > > something other than a multiple of 1K (+ RTE_PKTMBUF_HEADROOM), then the > > driver ends up silently programming the NIC to use a smaller max RX size > > than the caller specified. > > > > Should the driver error out in that case instead of only "sort of" > working? > > If not, it seems like it should be logging an error message. > > Well, it does log at the end of the whole rx init process what RX function > is > being used, so there is that. > However, looking at it now, given that there is an explicit setting in the > config > to request scattered mode, I think you may be right and that we should > error out > here if scattered mode is needed and not set. It could help prevent > unexpected > problems with these drivers. > +1. A hard fail at init if I've misconfigured things is much preferred to something that mostly works for typical test cases and only fails on corner/limit testing. > > 2) Second question is about the "/* It adds dual VLAN length for > supporting > > dual VLAN */" bit... > > > > As I understand it, dev_conf.rxmode.max_rx_pkt_len is supposed to be set > to > > the max frame size you support (although it says it's only used if jumbo > > frame is enabled). That same value is generally used when calculating the > > size that mbuf elements should be created at. > > > > The description for the data_room_size parameter of > > rte_pktmbuf_pool_create(): > > > > "Size of data buffer in each mbuf, including RTE_PKTMBUF_HEADROOM." > > > > > > If I support a max frame size of 9216 bytes (exactly a 1K multiple to > make > > the NIC happy), then max_rx_pkt_len is going to be 9216 and > data_room_size > > will be 9216 + RTE_PKTMBUF_HEADROOM. > > > > ixgbe_dev_rx_init() will calculate normalize that back to 9216, which > will > > fail the dual VLAN length check. The really nasty part about that is it > has > > a side-effect of enabling scattered RX regardless of the fact that I > didn't > > enable scattered RX in dev_conf.rxmode. > > > > The code in the e1000 PMD is similar, so nothing unique to ixgbe. > > > > Is that check correct? It seems wrong to be adding space for q-in-q on > top > > of your specified max frame size... > > The issue here is what the correct behaviour needs to be. If we have the > user > specify the maximum frame size including all vlan tags, then we hit the > problem > where we have to subtract the VLAN tag sizes when writing the value to the > NIC. > In that case, we will hit a problem where we get a e.g. 9210 byte frame - > to > reuse your example - without any VLAN tags, which will be rejected by the > hardware as being oversized. If we don't do the subtraction, and we get the > same 9210 byte packet with 2 VLAN tags on it, the hardware will accept it > and > then split it across multiple descriptors because the actual DMA size is > 9218 bytes. > As an app developer, I didn't realize the max frame size didn't include VLAN tags. I expected max frame size to be the size of the ethernet frame on the wire, which I would expect to include space used by any VLAN or MPLS tags. Is there anything in the docs or example apps about that? I did some digging as I was debugging this and didn't notice it, but entirely possible I just missed it. > I'm not sure there is a works-in-all-cases solution here. > Andriy's suggestion seems like it points in the right direction. >>From an app developer point of view, I'd expect to have a single max frame size value to track and the APIs should take care of any adjustments required internally. Maybe have rte_pktmbuf_pool_create() add the additional bytes when it calls rte_mempool_create() under the covers? Then it's nice and clean for the API without unexpected side-effects. Jay > /Bruce > > > > > Thanks, > > Jay >