Re: [PATCH net-next v6 3/3] net: axienet: Introduce dmaengine support

From: Florian Fainelli
Date: Mon Sep 18 2023 - 22:23:24 EST




On 9/18/2023 12:16 PM, Radhey Shyam Pandey wrote:
Add dmaengine framework to communicate with the xilinx DMAengine
driver(AXIDMA).

Axi ethernet driver uses separate channels for transmit and receive.
Add support for these channels to handle TX and RX with skb and
appropriate callbacks. Also add axi ethernet core interrupt for
dmaengine framework support.

The dmaengine framework was extended for metadata API support.
However it still needs further enhancements to make it well suited for
ethernet usecases. The ethernet features i.e ethtool set/get of DMA IP
properties, ndo_poll_controller,(mentioned in TODO) are not supported
and it requires follow-up discussions.

dmaengine support has a dependency on xilinx_dma as it uses
xilinx_vdma_channel_set_config() API to reset the DMA IP
which internally reset MAC prior to accessing MDIO.

Benchmark with netperf:

xilinx-zcu102-20232:~$ netperf -H 192.168.10.20 -t TCP_STREAM
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET
to 192.168.10.20 () port 0 AF_INET
Recv Send Send
Socket Socket Message Elapsed
Size Size Size Time Throughput
bytes bytes bytes secs. 10^6bits/sec

131072 16384 16384 10.03 915.55

xilinx-zcu102-20232:~$ netperf -H 192.168.10.20 -t UDP_STREAM
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET
to 192.168.10.20 () port 0 AF_INET
Socket Message Elapsed Messages
Size Size Time Okay Errors Throughput
bytes bytes secs # # 10^6bits/sec

212992 65507 10.00 18192 0 953.35
212992 10.00 18192 953.35

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
---
[snip]

Nice example of how to integrate an Ethernet driver with dmaengine, BTW.


+
+ /*Fill up app fields for checksum */

Missing space between * and Fill up, there are a few comments where it is the opposite where you don't add a space at the end before the *.

+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (lp->features & XAE_FEATURE_FULL_TX_CSUM) {
+ /* Tx Full Checksum Offload Enabled */
+ app[0] |= 2;
+ } else if (lp->features & XAE_FEATURE_PARTIAL_RX_CSUM) {

This is the transmit path here, is this path even taken?

+ csum_start_off = skb_transport_offset(skb);
+ csum_index_off = csum_start_off + skb->csum_offset;
+ /* Tx Partial Checksum Offload Enabled */
+ app[0] |= 1;
+ app[1] = (csum_start_off << 16) | csum_index_off;
+ }
+ } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
+ app[0] |= 2; /* Tx Full Checksum Offload Enabled */
+ }
+
+ dma_tx_desc = lp->tx_chan->device->device_prep_slave_sg(lp->tx_chan, skbuf_dma->sgl,
+ sg_len, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT, (void *)app);
+ if (!dma_tx_desc)
+ goto xmit_error_unmap_sg;
+
+ skbuf_dma->skb = skb;
+ skbuf_dma->sg_len = sg_len;
+ dma_tx_desc->callback_param = lp;
+ dma_tx_desc->callback_result = axienet_dma_tx_cb;
+ dmaengine_submit(dma_tx_desc);
+ dma_async_issue_pending(lp->tx_chan);
+
+ return NETDEV_TX_OK;
+
+xmit_error_unmap_sg:
+ dma_unmap_sg(lp->dev, skbuf_dma->sgl, sg_len, DMA_TO_DEVICE);
+ return NETDEV_TX_OK;
+}
+
/**
* axienet_tx_poll - Invoked once a transmit is completed by the
* Axi DMA Tx channel.
@@ -911,7 +1036,42 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
if (!lp->use_dmaengine)
return axienet_start_xmit_legacy(skb, ndev);
else
- return NETDEV_TX_BUSY;
+ return axienet_start_xmit_dmaengine(skb, ndev);
+}
+
+/**
+ * axienet_dma_rx_cb - DMA engine callback for RX channel.
+ * @data: Pointer to the skbuf_dma_descriptor structure.
+ * @result: error reporting through dmaengine_result.
+ * This function is called by dmaengine driver for RX channel to notify
+ * that the packet is received.
+ */
+static void axienet_dma_rx_cb(void *data, const struct dmaengine_result *result)
+{
+ struct axienet_local *lp = data;
+ struct skbuf_dma_descriptor *skbuf_dma;
+ size_t meta_len, meta_max_len, rx_len;
+ struct sk_buff *skb;
+ u32 *app;
+
+ skbuf_dma = axienet_get_rx_desc(lp, lp->rx_ring_tail++);
+ skb = skbuf_dma->skb;
+ app = dmaengine_desc_get_metadata_ptr(skbuf_dma->desc, &meta_len, &meta_max_len);

app might not be the best name, I understand this refers to a DMA application, in a broad sense that it is not specific, maybe cookie, or opaque_ptr would work better?

+ dma_unmap_single(lp->dev, skbuf_dma->dma_address, lp->max_frm_size,
+ DMA_FROM_DEVICE);
+ /* TODO: Derive app word index programmatically */
+ rx_len = (app[LEN_APP] & 0xFFFF);
+ skb_put(skb, rx_len);
+ skb->protocol = eth_type_trans(skb, lp->ndev);
+ skb->ip_summed = CHECKSUM_NONE;
+
+ netif_rx(skb);

Could save some cycles and call __netif_rx() here since AFAIR dmaengine uses a tasklet? netif_rx() is fine, just would have to compute need_bh_off.

+ u64_stats_update_begin(&lp->rx_stat_sync);
+ u64_stats_add(&lp->rx_packets, 1);
+ u64_stats_add(&lp->rx_bytes, rx_len);
+ u64_stats_update_end(&lp->rx_stat_sync);
+ axienet_rx_submit_desc(lp->ndev);
+ dma_async_issue_pending(lp->rx_chan);
}
/**
@@ -1147,6 +1307,142 @@ static irqreturn_t axienet_eth_irq(int irq, void *_ndev)
static void axienet_dma_err_handler(struct work_struct *work);
+/**
+ * axienet_rx_submit_desc - Submit the descriptors with required data
+ * like call backup API, skb buffer.. etc to dmaengine.
+ *
+ * @ndev: net_device pointer
+ *
+ *Return: 0, on success.
+ * non-zero error value on failure
+ */
+static int axienet_rx_submit_desc(struct net_device *ndev)
+{
+ struct dma_async_tx_descriptor *dma_rx_desc = NULL;
+ struct axienet_local *lp = netdev_priv(ndev);
+ struct skbuf_dma_descriptor *skbuf_dma;
+ struct sk_buff *skb;
+ dma_addr_t addr;
+ int ret;
+
+ skbuf_dma = axienet_get_rx_desc(lp, lp->rx_ring_head);
+ if (!skbuf_dma)
+ return -ENOMEM;
+ lp->rx_ring_head++;
+ skb = netdev_alloc_skb(ndev, lp->max_frm_size);
+ if (!skb)
+ return -ENOMEM;
+
+ sg_init_table(skbuf_dma->sgl, 1);
+ addr = dma_map_single(lp->dev, skb->data, lp->max_frm_size, DMA_FROM_DEVICE);

Need to check with dma_mapping_error() that the mapping succeeded.

Thanks!

pw-bot: cr
--
Florian