Re: [PATCH net v2 1/2] xsk: defer software checksum until the skb is complete

[Rongzheng Wang <testwrzwrz@gmail.com>]

Hi Maciej,

Thanks for the review.

Yes. xsk_skb_metadata() is only called for the first descriptor when
skb is NULL; continuation fragments do not call it. So
skb->ip_summed == CHECKSUM_PARTIAL means the first fragment requested
the checksum.

I'll preserve unlikely() around pool->tx_sw_csum like this:

    static int xsk_skb_sw_csum(struct sk_buff *skb,
                               struct xsk_buff_pool *pool)
    {
            if (unlikely(pool->tx_sw_csum &&
                         skb->ip_summed == CHECKSUM_PARTIAL))
                    return skb_checksum_help(skb);

            return 0;
    }

For the drop path: xsk_drop_skb() increments tx_invalid_descs and
consumes the skb; sendto() returns the error from skb_checksum_help().
Since the descriptors are already consumed, retrying is not useful. The
current metadata validation makes the malformed-packet path effectively
unreachable, so this is a defensive drop.

I verified the propagation in a local QEMU test kernel. For the test
only, I temporarily changed xsk_skb_sw_csum() to:

    static int xsk_skb_sw_csum(struct sk_buff *skb,
                               struct xsk_buff_pool *pool)
    {
            if (unlikely(pool->tx_sw_csum &&
                         skb->ip_summed == CHECKSUM_PARTIAL))
                    return -EINVAL;

            return 0;
    }

and ran test_xdp_metadata. It failed at the first generate_packet()
sendto:

    generate_packet:FAIL:sendto unexpected sendto: actual -1 < expected 0
    Summary: 0/0 PASSED, 0 SKIPPED, 1/0 FAILED
    TEST_RC=1

The normal kernel still passes. The test harness does not print errno,
so I can only state that sendto() returned -1; the injected error was
-EINVAL. I restored the source after the test.

Magnus suggested a distinct counter for malformed packets. I agree that
would be more precise than tx_invalid_descs, but I'd prefer to do that
in a separate patch instead of extending the UAPI in this stable fix.

I'll address the remaining feedback and send v3 as a new thread.

Thanks,
Rongzheng