| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f9099556-2360-41dc-8388-28a55dd2d588@gmail.com>
Date: Mon, 18 Sep 2023 19:23:16 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Radhey Shyam Pandey <radhey.shyam.pandey@....com>, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
michal.simek@....com, linux@...linux.org.uk
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
git@....com
Subject: Re: [PATCH net-next v6 3/3] net: axienet: Introduce dmaengine support
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@....com>
> ---
[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
Powered by blists - more mailing lists