Re: [PATCH] net: usb: ax88179_178a: add TSO feature

Jakub Kicinski
2021-11-13T03:48:01Z

On Fri, 12 Nov 2021 11:33:22 +0800 Jacky Chou wrote:
> On low-effciency embedded platforms, transmission performance is poor
> due to on Bulk-out with single packet.
> Adding TSO feature improves the transmission performance and reduces
> the number of interrupt caused by Bulk-out complete.
>
> Reference to module, net: usb: aqc111.
>
> Signed-off-by: Jacky Chou <jackychou@asix.com.tw>
> ---
> drivers/net/usb/ax88179_178a.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index c13167183..866954155 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1368,6 +1368,9 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> dev->net->needed_headroom = 8;
> dev->net->max_mtu = 4088;
>
> + if (usb_device_no_sg_constraint(dev->udev))
> + dev->can_dma_sg = 1;

Why don't you use this information...

> /* Initialize MII structure */
> dev->mii.dev = dev->net;
> dev->mii.mdio_read = ax88179_mdio_read;
> @@ -1377,11 +1380,14 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
> dev->mii.phy_id = 0x03;
> dev->mii.supports_gmii = 1;
>
> - dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> - NETIF_F_RXCSUM;
> + dev->net->features |= NETIF_F_SG | NETIF_F_IP_CSUM |
> + NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM | NETIF_F_TSO;

... to conditionally set the NETIF_F_SG flag?

> - dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> - NETIF_F_RXCSUM;
> + dev->net->hw_features |= NETIF_F_SG | NETIF_F_IP_CSUM |
> + NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM |
> + NETIF_F_TSO;

If you want to enable all the features just write:

dev->net->hw_features |= dev->net->features;

No need to repeat all the flags.

> + netif_set_gso_max_size(dev->net, 16384);
>
> /* Enable checksum offload */
> *tmp = AX_RXCOE_IP | AX_RXCOE_TCP | AX_RXCOE_UDP |
> @@ -1537,6 +1543,10 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
>
> headroom = skb_headroom(skb) - 8;
>
> + if (!dev->can_dma_sg && (dev->net->features & NETIF_F_SG) &&
> + skb_linearize(skb))
> + return NULL;
> +
> if ((skb_header_cloned(skb) || headroom < 0) &&
> pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
> dev_kfree_skb_any(skb);

Okay... where is the TSO implementation? The TCP stack passes
parameters like MSS which you need to inform the device about.
You should also count TSO packets as multiple packets in stats.

Are you sure this device supports TSO and not just SG?