From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1561AC352AA for ; Mon, 7 Oct 2019 11:05:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C9CA1206BB for ; Mon, 7 Oct 2019 11:05:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="zE4x2zFl" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727505AbfJGLFM (ORCPT ); Mon, 7 Oct 2019 07:05:12 -0400 Received: from mail-wr1-f67.google.com ([209.85.221.67]:42849 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727376AbfJGLFM (ORCPT ); Mon, 7 Oct 2019 07:05:12 -0400 Received: by mail-wr1-f67.google.com with SMTP id n14so14702097wrw.9 for ; Mon, 07 Oct 2019 04:05:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Zj8mqt4vHxRLIJeVsFDU1J1LNb4y9jZJA0Mimvx3lns=; b=zE4x2zFlQ7Ht8tmYJX7vwe7lVgK4MJiAHje27o9+Qk1591cPOjUpxCo07tXLmE9nkU rJ7688fTuiPvv5aUzHyIMcgQHoiT2XQ5Hhbkr0dgd+UR6fuigIBtA9adm36r7aqO0ZQE rmW1gCRBf56H4gN3oT8Yf3uCHwILiv3TKMPT3XLXvGNuhuqbuZsmEezlImjQeu3ZQlhj wZjnDsc+Migj0URT3hfByA0JsmFi3R6Fnm7YJG/APBscSULjRzTjf0F08mnrQ+lGh6dv WOVTPSxmyxaoDhtONNKm0hRjgUQeN9NGVtUhmT9gZKkotUDQQ3iPkKHr5quKsxAUWWMm /zXQ== 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:in-reply-to:user-agent; bh=Zj8mqt4vHxRLIJeVsFDU1J1LNb4y9jZJA0Mimvx3lns=; b=qU6Ldtn53EoL3vavqH21EQWCy6CndsIG3FcuWTY0rUc11aVgr4Zk/hefYI/tGWepvm mcswXEdt434hDfhcVbNOnwtSAYy2zUtBTWve1kHNH/pDnknOF51AmDwx13N3gyLOD4i2 JtunjPKfuwNI5O86MTdpFxuqYfD35otdWx5FwJ/IzQ418yf4PjuyEaeeNF1GW/dp0B+e 6/TniiDNVnmDYiDs22OWyaNda+wWxROGfTCCQgGaDIJcGLWXg2DyqHR+R1SRvuuYLEvg hQxfy4Vx+0BVTYBvk/d9AcWJL4EZhq6dKstep/CF6pbpyTHYZEcK9iVHoElcRI7X954s NoGg== X-Gm-Message-State: APjAAAXK3aN04DxT8RiZikU5Qn7V+kvknDD9Hd8qT4aRhBLo+AOyRcxj VyfkjFQcniu5xal6IXsOVfwHUg== X-Google-Smtp-Source: APXvYqz2YI6BSQa8sSXeaA/vwnBLHeBJNnIGReRRCB/0frIvBozAmtS+wGS2E0tipiCCk27gu64yVg== X-Received: by 2002:adf:8444:: with SMTP id 62mr21650364wrf.202.1570446308649; Mon, 07 Oct 2019 04:05:08 -0700 (PDT) Received: from apalos.home (ppp-94-65-93-45.home.otenet.gr. [94.65.93.45]) by smtp.gmail.com with ESMTPSA id h14sm23413963wro.44.2019.10.07.04.05.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Oct 2019 04:05:07 -0700 (PDT) Date: Mon, 7 Oct 2019 14:05:05 +0300 From: Ilias Apalodimas To: Lorenzo Bianconi Cc: netdev@vger.kernel.org, davem@davemloft.net, thomas.petazzoni@bootlin.com, brouer@redhat.com, lorenzo.bianconi@redhat.com, matteo.croce@redhat.com Subject: Re: [PATCH 3/7] net: mvneta: rely on build_skb in mvneta_rx_swbm poll routine Message-ID: <20191007110505.GA21878@apalos.home> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Lorenzo, On Sat, Oct 05, 2019 at 10:44:36PM +0200, Lorenzo Bianconi wrote: > Refactor mvneta_rx_swbm code introducing mvneta_swbm_rx_frame and > mvneta_swbm_add_rx_fragment routines. Rely on build_skb in order to > allocate skb since the previous patch introduced buffer recycling using > the page_pool API > > Tested-by: Ilias Apalodimas > Signed-off-by: Ilias Apalodimas > Signed-off-by: Jesper Dangaard Brouer > Signed-off-by: Lorenzo Bianconi > --- > drivers/net/ethernet/marvell/mvneta.c | 198 ++++++++++++++------------ > 1 file changed, 104 insertions(+), 94 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c > index 8beae0e1eda7..d775fcae9353 100644 > --- a/drivers/net/ethernet/marvell/mvneta.c > +++ b/drivers/net/ethernet/marvell/mvneta.c > @@ -323,6 +323,11 @@ > ETH_HLEN + ETH_FCS_LEN, \ > cache_line_size()) > > +#define MVNETA_SKB_PAD (SKB_DATA_ALIGN(sizeof(struct skb_shared_info) + \ > + NET_SKB_PAD)) > +#define MVNETA_SKB_SIZE(len) (SKB_DATA_ALIGN(len) + MVNETA_SKB_PAD) > +#define MVNETA_MAX_RX_BUF_SIZE (PAGE_SIZE - MVNETA_SKB_PAD) > + > #define IS_TSO_HEADER(txq, addr) \ > ((addr >= txq->tso_hdrs_phys) && \ > (addr < txq->tso_hdrs_phys + txq->size * TSO_HEADER_SIZE)) > @@ -646,7 +651,6 @@ static int txq_number = 8; > static int rxq_def; > > static int rx_copybreak __read_mostly = 256; > -static int rx_header_size __read_mostly = 128; > > /* HW BM need that each port be identify by a unique ID */ > static int global_port_id; > @@ -1941,30 +1945,102 @@ int mvneta_rx_refill_queue(struct mvneta_port *pp, struct mvneta_rx_queue *rxq) > return i; > } > > +static int > +mvneta_swbm_rx_frame(struct mvneta_port *pp, > + struct mvneta_rx_desc *rx_desc, > + struct mvneta_rx_queue *rxq, > + struct page *page) > +{ > + unsigned char *data = page_address(page); > + int data_len = -MVNETA_MH_SIZE, len; > + struct net_device *dev = pp->dev; > + enum dma_data_direction dma_dir; > + > + if (MVNETA_SKB_SIZE(rx_desc->data_size) > PAGE_SIZE) { > + len = MVNETA_MAX_RX_BUF_SIZE; > + data_len += len; > + } else { > + len = rx_desc->data_size; > + data_len += len - ETH_FCS_LEN; > + } > + > + dma_dir = page_pool_get_dma_dir(rxq->page_pool); > + dma_sync_single_range_for_cpu(dev->dev.parent, > + rx_desc->buf_phys_addr, 0, > + len, dma_dir); > + > + rxq->skb = build_skb(data, PAGE_SIZE); > + if (unlikely(!rxq->skb)) { > + netdev_err(dev, > + "Can't allocate skb on queue %d\n", > + rxq->id); > + dev->stats.rx_dropped++; > + rxq->skb_alloc_err++; > + return -ENOMEM; > + } > + page_pool_release_page(rxq->page_pool, page); > + > + skb_reserve(rxq->skb, MVNETA_MH_SIZE + NET_SKB_PAD); > + skb_put(rxq->skb, data_len); > + mvneta_rx_csum(pp, rx_desc->status, rxq->skb); > + > + rxq->left_size = rx_desc->data_size - len; > + rx_desc->buf_phys_addr = 0; > + > + return 0; > +} > + > +static void > +mvneta_swbm_add_rx_fragment(struct mvneta_port *pp, > + struct mvneta_rx_desc *rx_desc, > + struct mvneta_rx_queue *rxq, > + struct page *page) > +{ > + struct net_device *dev = pp->dev; > + enum dma_data_direction dma_dir; > + int data_len, len; > + > + if (rxq->left_size > MVNETA_MAX_RX_BUF_SIZE) { > + len = MVNETA_MAX_RX_BUF_SIZE; > + data_len = len; > + } else { > + len = rxq->left_size; > + data_len = len - ETH_FCS_LEN; > + } > + dma_dir = page_pool_get_dma_dir(rxq->page_pool); > + dma_sync_single_range_for_cpu(dev->dev.parent, > + rx_desc->buf_phys_addr, 0, > + len, dma_dir); > + if (data_len > 0) { > + /* refill descriptor with new buffer later */ > + skb_add_rx_frag(rxq->skb, > + skb_shinfo(rxq->skb)->nr_frags, > + page, NET_SKB_PAD, data_len, > + PAGE_SIZE); > + > + page_pool_release_page(rxq->page_pool, page); > + rx_desc->buf_phys_addr = 0; > + } > + rxq->left_size -= len; > +} > + > /* Main rx processing when using software buffer management */ > static int mvneta_rx_swbm(struct napi_struct *napi, > struct mvneta_port *pp, int budget, > struct mvneta_rx_queue *rxq) > { > - struct net_device *dev = pp->dev; > - int rx_todo, rx_proc; > - int refill = 0; > - u32 rcvd_pkts = 0; > - u32 rcvd_bytes = 0; > + int rcvd_pkts = 0, rcvd_bytes = 0; > + int rx_pending, refill, done = 0; > > /* Get number of received packets */ > - rx_todo = mvneta_rxq_busy_desc_num_get(pp, rxq); > - rx_proc = 0; > + rx_pending = mvneta_rxq_busy_desc_num_get(pp, rxq); > > /* Fairness NAPI loop */ > - while ((rcvd_pkts < budget) && (rx_proc < rx_todo)) { > + while (done < budget && done < rx_pending) { > struct mvneta_rx_desc *rx_desc = mvneta_rxq_next_desc_get(rxq); > unsigned char *data; > struct page *page; > - dma_addr_t phys_addr; > - u32 rx_status, index; > - int rx_bytes, skb_size, copy_size; > - int frag_num, frag_size, frag_offset; > + int index; > > index = rx_desc - rxq->descs; > page = (struct page *)rxq->buf_virt_addr[index]; > @@ -1972,98 +2048,33 @@ static int mvneta_rx_swbm(struct napi_struct *napi, > /* Prefetch header */ > prefetch(data); > > - phys_addr = rx_desc->buf_phys_addr; > - rx_status = rx_desc->status; > - rx_proc++; > rxq->refill_num++; > + done++; > + > + if (rx_desc->status & MVNETA_RXD_FIRST_DESC) { > + int err; > > - if (rx_status & MVNETA_RXD_FIRST_DESC) { > /* Check errors only for FIRST descriptor */ > - if (rx_status & MVNETA_RXD_ERR_SUMMARY) { > + if (rx_desc->status & MVNETA_RXD_ERR_SUMMARY) { > mvneta_rx_error(pp, rx_desc); > - dev->stats.rx_errors++; > + pp->dev->stats.rx_errors++; > /* leave the descriptor untouched */ > continue; > } > - rx_bytes = rx_desc->data_size - > - (ETH_FCS_LEN + MVNETA_MH_SIZE); > > - /* Allocate small skb for each new packet */ > - skb_size = max(rx_copybreak, rx_header_size); > - rxq->skb = netdev_alloc_skb_ip_align(dev, skb_size); > - if (unlikely(!rxq->skb)) { > - netdev_err(dev, > - "Can't allocate skb on queue %d\n", > - rxq->id); > - dev->stats.rx_dropped++; > - rxq->skb_alloc_err++; > + err = mvneta_swbm_rx_frame(pp, rx_desc, rxq, page); > + if (err) > continue; The error is practically a build_skb failure probably due to low memory. I think it's better to break the loop and let the system reclaim some of it's free memory instead of continuing to process (and allocate more skb's) > - } > - copy_size = min(skb_size, rx_bytes); > - > - /* Copy data from buffer to SKB, skip Marvell header */ > - memcpy(rxq->skb->data, data + MVNETA_MH_SIZE, > - copy_size); > - skb_put(rxq->skb, copy_size); > - rxq->left_size = rx_bytes - copy_size; > - > - mvneta_rx_csum(pp, rx_status, rxq->skb); > - if (rxq->left_size == 0) { > - int size = copy_size + MVNETA_MH_SIZE; > - > - dma_sync_single_range_for_cpu(dev->dev.parent, > - phys_addr, 0, > - size, > - DMA_FROM_DEVICE); > - > - /* leave the descriptor and buffer untouched */ > - } else { > - /* refill descriptor with new buffer later */ > - rx_desc->buf_phys_addr = 0; > - > - frag_num = 0; > - frag_offset = copy_size + MVNETA_MH_SIZE; > - frag_size = min(rxq->left_size, > - (int)(PAGE_SIZE - frag_offset)); > - skb_add_rx_frag(rxq->skb, frag_num, page, > - frag_offset, frag_size, > - PAGE_SIZE); > - page_pool_release_page(rxq->page_pool, page); > - rxq->left_size -= frag_size; > - } > } else { > - /* Middle or Last descriptor */ > if (unlikely(!rxq->skb)) { > pr_debug("no skb for rx_status 0x%x\n", > - rx_status); > + rx_desc->status); > continue; > } > - if (!rxq->left_size) { > - /* last descriptor has only FCS */ > - /* and can be discarded */ > - dma_sync_single_range_for_cpu(dev->dev.parent, > - phys_addr, 0, > - ETH_FCS_LEN, > - DMA_FROM_DEVICE); > - /* leave the descriptor and buffer untouched */ > - } else { > - /* refill descriptor with new buffer later */ > - rx_desc->buf_phys_addr = 0; > - > - frag_num = skb_shinfo(rxq->skb)->nr_frags; > - frag_offset = 0; > - frag_size = min(rxq->left_size, > - (int)(PAGE_SIZE - frag_offset)); > - skb_add_rx_frag(rxq->skb, frag_num, page, > - frag_offset, frag_size, > - PAGE_SIZE); > - > - page_pool_release_page(rxq->page_pool, page); > - rxq->left_size -= frag_size; > - } > + mvneta_swbm_add_rx_fragment(pp, rx_desc, rxq, page); > } /* Middle or Last descriptor */ > > - if (!(rx_status & MVNETA_RXD_LAST_DESC)) > + if (!(rx_desc->status & MVNETA_RXD_LAST_DESC)) > /* no last descriptor this time */ > continue; > > @@ -2079,13 +2090,12 @@ static int mvneta_rx_swbm(struct napi_struct *napi, > rcvd_bytes += rxq->skb->len; > > /* Linux processing */ > - rxq->skb->protocol = eth_type_trans(rxq->skb, dev); > + rxq->skb->protocol = eth_type_trans(rxq->skb, pp->dev); > > napi_gro_receive(napi, rxq->skb); > > /* clean uncomplete skb pointer in queue */ > rxq->skb = NULL; > - rxq->left_size = 0; > } > > mvneta_update_stats(pp, rcvd_pkts, rcvd_bytes, false); > @@ -2094,7 +2104,7 @@ static int mvneta_rx_swbm(struct napi_struct *napi, > refill = mvneta_rx_refill_queue(pp, rxq); > > /* Update rxq management counters */ > - mvneta_rxq_desc_num_update(pp, rxq, rx_proc, refill); > + mvneta_rxq_desc_num_update(pp, rxq, done, refill); > > return rcvd_pkts; > } > @@ -2945,7 +2955,7 @@ static void mvneta_rxq_hw_init(struct mvneta_port *pp, > /* Set Offset */ > mvneta_rxq_offset_set(pp, rxq, 0); > mvneta_rxq_buf_size_set(pp, rxq, PAGE_SIZE < SZ_64K ? > - PAGE_SIZE : > + MVNETA_MAX_RX_BUF_SIZE : > MVNETA_RX_BUF_SIZE(pp->pkt_size)); > mvneta_rxq_bm_disable(pp, rxq); > mvneta_rxq_fill(pp, rxq, rxq->size); > @@ -4655,7 +4665,7 @@ static int mvneta_probe(struct platform_device *pdev) > SET_NETDEV_DEV(dev, &pdev->dev); > > pp->id = global_port_id++; > - pp->rx_offset_correction = 0; /* not relevant for SW BM */ > + pp->rx_offset_correction = NET_SKB_PAD; > > /* Obtain access to BM resources if enabled and already initialized */ > bm_node = of_parse_phandle(dn, "buffer-manager", 0); > -- > 2.21.0 > Regards /Ilias