Re: [PATCH v6 RESEND net-next 4/5] net: enetc: add LSO support for i.MX95 ENETC PF

[Simon Horman]

On Fri, Dec 06, 2024 at 09:59:38AM +0000, Simon Horman wrote:
> On Wed, Dec 04, 2024 at 01:29:31PM +0800, Wei Fang wrote:
> > ENETC rev 4.1 supports large send offload (LSO), segmenting large TCP
> > and UDP transmit units into multiple Ethernet frames. To support LSO,
> > software needs to fill some auxiliary information in Tx BD, such as LSO
> > header length, frame length, LSO maximum segment size, etc.
> > 
> > At 1Gbps link rate, TCP segmentation was tested using iperf3, and the
> > CPU performance before and after applying the patch was compared through
> > the top command. It can be seen that LSO saves a significant amount of
> > CPU cycles compared to software TSO.
> > 
> > Before applying the patch:
> > %Cpu(s):  0.1 us,  4.1 sy,  0.0 ni, 85.7 id,  0.0 wa,  0.5 hi,  9.7 si
> > 
> > After applying the patch:
> > %Cpu(s):  0.1 us,  2.3 sy,  0.0 ni, 94.5 id,  0.0 wa,  0.4 hi,  2.6 si
> > 
> > Signed-off-by: Wei Fang <wei.fang@xxxxxxx>
> > Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> > Reviewed-by: Claudiu Manoil <claudiu.manoil@xxxxxxx>
> > ---
> > v2: no changes
> > v3: use enetc_skb_is_ipv6() helper fucntion which is added in patch 2
> > v4: fix a typo
> > v5: no changes
> > v6: remove error logs from the datapath
> > ---
> >  drivers/net/ethernet/freescale/enetc/enetc.c  | 259 +++++++++++++++++-
> >  drivers/net/ethernet/freescale/enetc/enetc.h  |  15 +
> >  .../net/ethernet/freescale/enetc/enetc4_hw.h  |  22 ++
> >  .../net/ethernet/freescale/enetc/enetc_hw.h   |  15 +-
> >  .../freescale/enetc/enetc_pf_common.c         |   3 +
> >  5 files changed, 304 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> > index dafe7aeac26b..82a7932725f9 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > @@ -523,6 +523,226 @@ static void enetc_tso_complete_csum(struct enetc_bdr *tx_ring, struct tso_t *tso
> >  	}
> >  }
> >  
> > +static inline int enetc_lso_count_descs(const struct sk_buff *skb)
> > +{
> > +	/* 4 BDs: 1 BD for LSO header + 1 BD for extended BD + 1 BD
> > +	 * for linear area data but not include LSO header, namely
> > +	 * skb_headlen(skb) - lso_hdr_len. And 1 BD for gap.
> > +	 */
> > +	return skb_shinfo(skb)->nr_frags + 4;
> > +}
> > +
> > +static int enetc_lso_get_hdr_len(const struct sk_buff *skb)
> > +{
> > +	int hdr_len, tlen;
> > +
> > +	tlen = skb_is_gso_tcp(skb) ? tcp_hdrlen(skb) : sizeof(struct udphdr);
> > +	hdr_len = skb_transport_offset(skb) + tlen;
> 
> Hi Wei,
> 
> I am wondering if packets that are neither TCP nor UDP can be process
> by the LSO code added by this patch, and if so, what the implications are.

Sorry, I now realise that the answer to that is rather obvious: no
due to feature flags. I should have paid more attention to patch 5/5
before sending the above.

> 
> > +
> > +	return hdr_len;
> > +}
> 
> ...
>