RE: [PATCH net-next v7 3/3] net: axienet: Introduce dmaengine support

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@xxxxxxxxx>
> Sent: Tuesday, October 10, 2023 1:51 AM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Simek, Michal
> <michal.simek@xxxxxxx>; linux@xxxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; git (AMD-
> Xilinx) <git@xxxxxxx>
> Subject: Re: [PATCH net-next v7 3/3] net: axienet: Introduce dmaengine
> support
> 
> On 9/27/23 12:32, 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]
> >   /**
> >    * axienet_tx_poll - Invoked once a transmit is completed by the
> >    * Axi DMA Tx channel.
> > @@ -911,7 +1036,43 @@ 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);
> 
> Did not notice this before, but should consider using a separate set of
> net_device_ops with a different ndo_start_xmit() implementation, rather
> than add another layer of indirection here.

Thanks, will make it a separate net_device_ops for dmaengine flow and
revert changes in axienet_start_xmit() from 2/3 patch.

Thanks,
Radhey