RE: [PATCH v4 net-next 2/5] net: enetc: add Tx checksum offload for i.MX95 ENETC
From: Wei Fang
Date: Sun Nov 17 2024 - 20:23:37 EST
> On Fri, 15 Nov 2024 10:47:41 +0800 Wei Fang wrote:
> > +static inline bool enetc_skb_is_ipv6(struct sk_buff *skb) {
> > + return vlan_get_protocol(skb) == htons(ETH_P_IPV6); }
> > +
> > +static inline bool enetc_skb_is_tcp(struct sk_buff *skb) {
> > + return skb->csum_offset == offsetof(struct tcphdr, check); }
>
> Please don't use "inline" for trivial functions, compiler will inline them anyway,
> and it hides unused code. In addition to being pointless.
>
> > static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct
> > sk_buff *skb) {
> > bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
> > @@ -160,6 +181,27 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> > dma_addr_t dma;
> > u8 flags = 0;
> >
> > + enetc_clear_tx_bd(&temp_bd);
> > + if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > + /* Can not support TSD and checksum offload at the same time */
> > + if (priv->active_offloads & ENETC_F_TXCSUM &&
> > + enetc_tx_csum_offload_check(skb) && !tx_ring->tsd_enable) {
> > + temp_bd.l3_start = skb_network_offset(skb);
> > + temp_bd.ipcs = enetc_skb_is_ipv6(skb) ? 0 : 1;
>
> Linux calculates the IPv4 csum, always, no need.
>
> > + temp_bd.l3_hdr_size = skb_network_header_len(skb) / 4;
> > + temp_bd.l3t = enetc_skb_is_ipv6(skb) ? 1 : 0;
>
> no need for ternary op, simply :
>
> temp_bd.l3t = enetc_skb_is_ipv6(skb);
>
> > + temp_bd.l4t = enetc_skb_is_tcp(skb) ? ENETC_TXBD_L4T_TCP :
> > + ENETC_TXBD_L4T_UDP;
> > + flags |= ENETC_TXBD_FLAGS_CSUM_LSO |
> ENETC_TXBD_FLAGS_L4CS;
> > + } else {
> > + if (skb_checksum_help(skb)) {
> > + dev_err(tx_ring->dev, "skb_checksum_help() error\n");
> > +
> > + return 0;
>
> don't print errors on the datapath, it may flood the logs
Thanks for the comments, I will improve this patch.
> --
> pw-bot: cr