[xdp-hints] Re: [RFC bpf-next v3 09/14] net/mlx5e: Implement devtx kfuncs

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Fri, Jul 07, 2023 at 12:30:01PM -0700, Stanislav Fomichev wrote:
> +
> +static int mlx5e_devtx_request_l4_checksum(const struct devtx_ctx *_ctx,
> +					   u16 csum_start, u16 csum_offset)
> +{
> +	const struct mlx5e_devtx_ctx *ctx = (void *)_ctx;
> +	struct mlx5_wqe_eth_seg *eseg;
> +
> +	if (unlikely(!ctx->wqe))
> +		return -ENODATA;
> +
> +	eseg = &ctx->wqe->eth;
> +
> +	switch (csum_offset) {
> +	case sizeof(struct ethhdr) + sizeof(struct iphdr) + offsetof(struct udphdr, check):
> +	case sizeof(struct ethhdr) + sizeof(struct ipv6hdr) + offsetof(struct udphdr, check):
> +		/* Looks like HW/FW is doing parsing, so offsets are largely ignored. */
> +		eseg->cs_flags = MLX5_ETH_WQE_L3_CSUM | MLX5_ETH_WQE_L4_CSUM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

I think this proves my point: csum is not generalizable even across veth and mlx5.
Above is a square peg that tries to fit csum_start/offset api (that makes sense from SW pov)
into HW that has different ideas about csum-ing.

Here is what mlx5 does:
mlx5e_txwqe_build_eseg_csum(struct mlx5e_txqsq *sq, struct sk_buff *skb,
                            struct mlx5e_accel_tx_state *accel,
                            struct mlx5_wqe_eth_seg *eseg)
{
        if (unlikely(mlx5e_ipsec_txwqe_build_eseg_csum(sq, skb, eseg)))
                return;

        if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
                eseg->cs_flags = MLX5_ETH_WQE_L3_CSUM;
                if (skb->encapsulation) {
                        eseg->cs_flags |= MLX5_ETH_WQE_L3_INNER_CSUM |
                                          MLX5_ETH_WQE_L4_INNER_CSUM;
                        sq->stats->csum_partial_inner++;
                } else {
                        eseg->cs_flags |= MLX5_ETH_WQE_L4_CSUM;
                        sq->stats->csum_partial++;
                }

How would you generalize that into csum api that will work across NICs ?

My answer stands: you cannot.

My proposal again:
add driver specifc kfuncs and hooks for things like csum.

Kuba,
since you nacked driver specific stuff please suggest a way to unblock this stalemate.