Re: [PATCH 2/2] net: emac: implement TCP TSO

[Florian Fainelli <f.fainelli@gmail.com>]

On 10/17/2018 12:53 PM, Christian Lamparter wrote:
> This patch enables TSO(v4) hw feature for emac driver.
> As atleast the APM82181's TCP/IP acceleration hardware
> controller (TAH) provides TCP segmentation support in
> the transmit path.
> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  drivers/net/ethernet/ibm/emac/core.c | 101 ++++++++++++++++++++++++++-
>  drivers/net/ethernet/ibm/emac/core.h |   4 ++
>  drivers/net/ethernet/ibm/emac/emac.h |   7 ++
>  drivers/net/ethernet/ibm/emac/tah.c  |  20 ++++++
>  drivers/net/ethernet/ibm/emac/tah.h  |   2 +
>  5 files changed, 133 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> index be560f9031f4..49ffbd6e1707 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> @@ -38,6 +38,9 @@
>  #include <linux/mii.h>
>  #include <linux/bitops.h>
>  #include <linux/if_vlan.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
>  #include <linux/workqueue.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> @@ -1410,6 +1413,52 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
>  	return 0;
>  }
>  
> +const u32 tah_ss[TAH_NO_SSR] = { 9000, 4500, 1500, 1300, 576, 176 };
> +
> +static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
> +		       u16 *ctrl)
> +{
> +	if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) &&
> +	    skb_is_gso(skb) && !!(skb_shinfo(skb)->gso_type &
> +				(SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
> +		u32 seg_size = 0, i;
> +
> +		/* Get the MTU */
> +		seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb)
> +			+ skb_network_header_len(skb);
> +
> +		/* Restriction applied for the segmentation size
> +		 * to use HW segmentation offload feature: the size
> +		 * of the segment must not be less than 168 bytes for
> +		 * DIX formatted segments, or 176 bytes for
> +		 * IEEE formatted segments.
> +		 *
> +		 * I use value 176 to check for the segment size here
> +		 * as it can cover both 2 conditions above.
> +		 */
> +		if (seg_size < 176)
> +			return -ENODEV;
> +
> +		/* Get the best suitable MTU */
> +		for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
> +			u32 curr_seg = tah_ss[i];
> +
> +			if (curr_seg > dev->ndev->mtu ||
> +			    curr_seg > seg_size)
> +				continue;
> +
> +			*ctrl &= ~EMAC_TX_CTRL_TAH_CSUM;
> +			*ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
> +			return 0;

This is something that you can possibly take out of your hot path and
recalculate when the MTU actually changes?

[snip]

> +static netdev_tx_t emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct emac_instance *dev = netdev_priv(ndev);
> +	struct sk_buff *segs, *curr;
> +
> +	segs = skb_gso_segment(skb, ndev->features &
> +					~(NETIF_F_TSO | NETIF_F_TSO6));
> +	if (IS_ERR_OR_NULL(segs)) {
> +		goto drop;
> +	} else {
> +		while (segs) {
> +			/* check for overflow */
> +			if (dev->tx_cnt >= NUM_TX_BUFF) {
> +				dev_kfree_skb_any(segs);
> +				goto drop;
> +			}

Would setting dev->max_gso_segs somehow help make sure the stack does
not feed you oversized GSO'd skbs?
-- 
Florian