Re: [PATCH v4 net-next 2/5] net: enetc: add Tx checksum offload for i.MX95 ENETC

[Jakub Kicinski]

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
-- 
pw-bot: cr