From owner-svn-src-all@freebsd.org Mon Feb 24 15:35:32 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 6BD8523800E; Mon, 24 Feb 2020 15:35:32 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48R5jw2GCHz4FM2; Mon, 24 Feb 2020 15:35:32 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 442CC2763A; Mon, 24 Feb 2020 15:35:32 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 01OFZWeA092594; Mon, 24 Feb 2020 15:35:32 GMT (envelope-from mw@FreeBSD.org) Received: (from mw@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 01OFZVfi092591; Mon, 24 Feb 2020 15:35:31 GMT (envelope-from mw@FreeBSD.org) Message-Id: <202002241535.01OFZVfi092591@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mw set sender to mw@FreeBSD.org using -f From: Marcin Wojtas Date: Mon, 24 Feb 2020 15:35:31 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r358289 - head/sys/dev/ena X-SVN-Group: head X-SVN-Commit-Author: mw X-SVN-Commit-Paths: head/sys/dev/ena X-SVN-Commit-Revision: 358289 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Feb 2020 15:35:32 -0000 Author: mw Date: Mon Feb 24 15:35:31 2020 New Revision: 358289 URL: https://svnweb.freebsd.org/changeset/base/358289 Log: Rework and simplify Tx DMA mapping in ENA Driver working in LLQ mode in some cases can send only few last segments of the mbuf using DMA engine, and the rest of them are sent to the device using direct PCI transaction. To map the only necessary data, two DMA maps were used. That solution was very rough and was causing a bug - if both maps were used (head_map and seg_map), there was a race in between two flows on two queues and the device was receiving corrupted data which could be further received on the other host if the Tx cksum offload was enabled. As it's ok to map whole mbuf and then send to the device only needed segments, the design was simplified to use only single DMA map. The driver version was updated to v2.1.1 as it's important bug fix. Submitted by: Michal Krawczyk Obtained from: Semihalf MFC after: 2 weeks Sponsored by: Amazon, Inc. Modified: head/sys/dev/ena/ena.c head/sys/dev/ena/ena.h head/sys/dev/ena/ena_datapath.c Modified: head/sys/dev/ena/ena.c ============================================================================== --- head/sys/dev/ena/ena.c Mon Feb 24 12:35:58 2020 (r358288) +++ head/sys/dev/ena/ena.c Mon Feb 24 15:35:31 2020 (r358289) @@ -558,15 +558,10 @@ ena_release_all_tx_dmamap(struct ena_ring *tx_ring) } } #endif /* DEV_NETMAP */ - if (tx_info->map_head != NULL) { - bus_dmamap_destroy(tx_tag, tx_info->map_head); - tx_info->map_head = NULL; + if (tx_info->dmamap != NULL) { + bus_dmamap_destroy(tx_tag, tx_info->dmamap); + tx_info->dmamap = NULL; } - - if (tx_info->map_seg != NULL) { - bus_dmamap_destroy(tx_tag, tx_info->map_seg); - tx_info->map_seg = NULL; - } } } @@ -627,25 +622,14 @@ ena_setup_tx_resources(struct ena_adapter *adapter, in /* ... and create the buffer DMA maps */ for (i = 0; i < tx_ring->ring_size; i++) { err = bus_dmamap_create(adapter->tx_buf_tag, 0, - &tx_ring->tx_buffer_info[i].map_head); + &tx_ring->tx_buffer_info[i].dmamap); if (unlikely(err != 0)) { ena_trace(ENA_ALERT, - "Unable to create Tx DMA map_head for buffer %d\n", + "Unable to create Tx DMA map for buffer %d\n", i); goto err_map_release; } - tx_ring->tx_buffer_info[i].seg_mapped = false; - err = bus_dmamap_create(adapter->tx_buf_tag, 0, - &tx_ring->tx_buffer_info[i].map_seg); - if (unlikely(err != 0)) { - ena_trace(ENA_ALERT, - "Unable to create Tx DMA map_seg for buffer %d\n", - i); - goto err_map_release; - } - tx_ring->tx_buffer_info[i].head_mapped = false; - #ifdef DEV_NETMAP if (adapter->ifp->if_capenable & IFCAP_NETMAP) { map = tx_ring->tx_buffer_info[i].nm_info.map_seg; @@ -720,28 +704,13 @@ ena_free_tx_resources(struct ena_adapter *adapter, int /* Free buffer DMA maps, */ for (int i = 0; i < tx_ring->ring_size; i++) { - if (tx_ring->tx_buffer_info[i].head_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_head, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_head); - tx_ring->tx_buffer_info[i].head_mapped = false; - } + bus_dmamap_sync(adapter->tx_buf_tag, + tx_ring->tx_buffer_info[i].dmamap, BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(adapter->tx_buf_tag, + tx_ring->tx_buffer_info[i].dmamap); bus_dmamap_destroy(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_head); + tx_ring->tx_buffer_info[i].dmamap); - if (tx_ring->tx_buffer_info[i].seg_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_seg, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_seg); - tx_ring->tx_buffer_info[i].seg_mapped = false; - } - bus_dmamap_destroy(adapter->tx_buf_tag, - tx_ring->tx_buffer_info[i].map_seg); - #ifdef DEV_NETMAP if (adapter->ifp->if_capenable & IFCAP_NETMAP) { nm_info = &tx_ring->tx_buffer_info[i].nm_info; @@ -1209,21 +1178,9 @@ ena_free_tx_bufs(struct ena_adapter *adapter, unsigned qid, i); } - if (tx_info->head_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_head, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_info->map_head); - tx_info->head_mapped = false; - } - - if (tx_info->seg_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_seg, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_info->map_seg); - tx_info->seg_mapped = false; - } + bus_dmamap_sync(adapter->tx_buf_tag, tx_info->dmamap, + BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(adapter->tx_buf_tag, tx_info->dmamap); m_free(tx_info->mbuf); tx_info->mbuf = NULL; Modified: head/sys/dev/ena/ena.h ============================================================================== --- head/sys/dev/ena/ena.h Mon Feb 24 12:35:58 2020 (r358288) +++ head/sys/dev/ena/ena.h Mon Feb 24 15:35:31 2020 (r358289) @@ -41,7 +41,7 @@ #define DRV_MODULE_VER_MAJOR 2 #define DRV_MODULE_VER_MINOR 1 -#define DRV_MODULE_VER_SUBMINOR 0 +#define DRV_MODULE_VER_SUBMINOR 1 #define DRV_MODULE_NAME "ena" @@ -241,13 +241,8 @@ struct ena_tx_buffer { unsigned int tx_descs; /* # of buffers used by this mbuf */ unsigned int num_of_bufs; - bus_dmamap_t map_head; - bus_dmamap_t map_seg; - /* Indicate if segments of the mbuf were mapped */ - bool seg_mapped; - /* Indicate if bufs[0] maps the linear data of the mbuf */ - bool head_mapped; + bus_dmamap_t dmamap; /* Used to detect missing tx packets */ struct bintime timestamp; Modified: head/sys/dev/ena/ena_datapath.c ============================================================================== --- head/sys/dev/ena/ena_datapath.c Mon Feb 24 12:35:58 2020 (r358288) +++ head/sys/dev/ena/ena_datapath.c Mon Feb 24 15:35:31 2020 (r358289) @@ -52,7 +52,6 @@ static inline void ena_rx_checksum(struct ena_ring *, static void ena_tx_csum(struct ena_com_tx_ctx *, struct mbuf *); static int ena_check_and_collapse_mbuf(struct ena_ring *tx_ring, struct mbuf **mbuf); -static void ena_dmamap_llq(void *, bus_dma_segment_t *, int, int); static int ena_xmit_mbuf(struct ena_ring *, struct mbuf **); static void ena_start_xmit(struct ena_ring *); @@ -263,21 +262,10 @@ ena_tx_cleanup(struct ena_ring *tx_ring) tx_info->mbuf = NULL; bintime_clear(&tx_info->timestamp); - /* Map is no longer required */ - if (tx_info->head_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_head, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_info->map_head); - tx_info->head_mapped = false; - } - if (tx_info->seg_mapped == true) { - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_seg, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(adapter->tx_buf_tag, - tx_info->map_seg); - tx_info->seg_mapped = false; - } + bus_dmamap_sync(adapter->tx_buf_tag, tx_info->dmamap, + BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(adapter->tx_buf_tag, + tx_info->dmamap); ena_trace(ENA_DBG | ENA_TXPTH, "tx: q %d mbuf %p completed\n", tx_ring->qid, mbuf); @@ -814,22 +802,6 @@ ena_check_and_collapse_mbuf(struct ena_ring *tx_ring, return (0); } -static void -ena_dmamap_llq(void *arg, bus_dma_segment_t *segs, int nseg, int error) -{ - struct ena_com_buf *ena_buf = arg; - - if (unlikely(error != 0)) { - ena_buf->paddr = 0; - return; - } - - KASSERT(nseg == 1, ("Invalid num of segments for LLQ dma")); - - ena_buf->paddr = segs->ds_addr; - ena_buf->len = segs->ds_len; -} - static int ena_tx_map_mbuf(struct ena_ring *tx_ring, struct ena_tx_buffer *tx_info, struct mbuf *mbuf, void **push_hdr, u16 *header_len) @@ -837,15 +809,29 @@ ena_tx_map_mbuf(struct ena_ring *tx_ring, struct ena_t struct ena_adapter *adapter = tx_ring->adapter; struct ena_com_buf *ena_buf; bus_dma_segment_t segs[ENA_BUS_DMA_SEGS]; + size_t iseg = 0; uint32_t mbuf_head_len, frag_len; uint16_t push_len = 0; uint16_t delta = 0; - int i, rc, nsegs; + int rc, nsegs; mbuf_head_len = mbuf->m_len; tx_info->mbuf = mbuf; ena_buf = tx_info->bufs; + /* + * For easier maintaining of the DMA map, map the whole mbuf even if + * the LLQ is used. The descriptors will be filled using the segments. + */ + rc = bus_dmamap_load_mbuf_sg(adapter->tx_buf_tag, tx_info->dmamap, mbuf, + segs, &nsegs, BUS_DMA_NOWAIT); + if (unlikely((rc != 0) || (nsegs == 0))) { + ena_trace(ENA_WARNING, + "dmamap load failed! err: %d nsegs: %d\n", rc, nsegs); + goto dma_error; + } + + if (tx_ring->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV) { /* * When the device is LLQ mode, the driver will copy @@ -885,19 +871,16 @@ ena_tx_map_mbuf(struct ena_ring *tx_ring, struct ena_t * in the first mbuf of the mbuf chain. */ if (mbuf_head_len > push_len) { - rc = bus_dmamap_load(adapter->tx_buf_tag, - tx_info->map_head, - mbuf->m_data + push_len, mbuf_head_len - push_len, - ena_dmamap_llq, ena_buf, BUS_DMA_NOWAIT); - if (unlikely((rc != 0) || (ena_buf->paddr == 0))) - goto single_dma_error; - + ena_buf->paddr = segs[iseg].ds_addr + push_len; + ena_buf->len = segs[iseg].ds_len - push_len; ena_buf++; tx_info->num_of_bufs++; - - tx_info->head_mapped = true; } - mbuf = mbuf->m_next; + /* + * Advance the seg index as either the 1st mbuf was mapped or is + * a part of push_hdr. + */ + iseg++; } else { *push_hdr = NULL; /* @@ -918,7 +901,7 @@ ena_tx_map_mbuf(struct ena_ring *tx_ring, struct ena_t * If LLQ is not supported, the loop will be skipped. */ while (delta > 0) { - frag_len = mbuf->m_len; + frag_len = segs[iseg].ds_len; /* * If whole segment contains header just move to the @@ -931,50 +914,32 @@ ena_tx_map_mbuf(struct ena_ring *tx_ring, struct ena_t * Map rest of the packet data that was contained in * the mbuf. */ - rc = bus_dmamap_load(adapter->tx_buf_tag, - tx_info->map_head, mbuf->m_data + delta, - frag_len - delta, ena_dmamap_llq, ena_buf, - BUS_DMA_NOWAIT); - if (unlikely((rc != 0) || (ena_buf->paddr == 0))) - goto single_dma_error; - + ena_buf->paddr = segs[iseg].ds_addr + delta; + ena_buf->len = frag_len - delta; ena_buf++; tx_info->num_of_bufs++; - tx_info->head_mapped = true; delta = 0; } - - mbuf = mbuf->m_next; + iseg++; } if (mbuf == NULL) { return (0); } - /* Map rest of the mbufs */ - rc = bus_dmamap_load_mbuf_sg(adapter->tx_buf_tag, tx_info->map_seg, mbuf, - segs, &nsegs, BUS_DMA_NOWAIT); - if (unlikely((rc != 0) || (nsegs == 0))) { - ena_trace(ENA_WARNING, - "dmamap load failed! err: %d nsegs: %d\n", rc, nsegs); - goto dma_error; - } - - for (i = 0; i < nsegs; i++) { - ena_buf->len = segs[i].ds_len; - ena_buf->paddr = segs[i].ds_addr; + /* Map rest of the mbuf */ + while (iseg < nsegs) { + ena_buf->paddr = segs[iseg].ds_addr; + ena_buf->len = segs[iseg].ds_len; ena_buf++; + iseg++; + tx_info->num_of_bufs++; } - tx_info->num_of_bufs += nsegs; - tx_info->seg_mapped = true; return (0); dma_error: - if (tx_info->head_mapped == true) - bus_dmamap_unload(adapter->tx_buf_tag, tx_info->map_head); -single_dma_error: counter_u64_add(tx_ring->tx_stats.dma_mapping_err, 1); tx_info->mbuf = NULL; return (rc); @@ -1100,25 +1065,14 @@ ena_xmit_mbuf(struct ena_ring *tx_ring, struct mbuf ** } } - if (tx_info->head_mapped == true) - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_head, - BUS_DMASYNC_PREWRITE); - if (tx_info->seg_mapped == true) - bus_dmamap_sync(adapter->tx_buf_tag, tx_info->map_seg, - BUS_DMASYNC_PREWRITE); + bus_dmamap_sync(adapter->tx_buf_tag, tx_info->dmamap, + BUS_DMASYNC_PREWRITE); return (0); dma_error: tx_info->mbuf = NULL; - if (tx_info->seg_mapped == true) { - bus_dmamap_unload(adapter->tx_buf_tag, tx_info->map_seg); - tx_info->seg_mapped = false; - } - if (tx_info->head_mapped == true) { - bus_dmamap_unload(adapter->tx_buf_tag, tx_info->map_head); - tx_info->head_mapped = false; - } + bus_dmamap_unload(adapter->tx_buf_tag, tx_info->dmamap); return (rc); }