Re: [Patch net v3] mlx5: force CHECKSUM_NONE for short ethernet frames

On Tue, 2018-12-04 at 12:35 -0800, Cong Wang wrote:

On Tue, Dec 4, 2018 at 11:17 AM Saeed Mahameed
[off-list ref] wrote:

quoted

On Mon, Dec 3, 2018 at 11:52 PM Eric Dumazet [off-list ref]
wrote:

quoted

On Mon, Dec 3, 2018 at 11:30 PM Cong Wang <
xiyou.wangcong@gmail.com> wrote:

quoted

On Mon, Dec 3, 2018 at 11:08 PM Eric Dumazet <
edumazet@google.com> wrote:

quoted

The hardware has probably validated the L3 & L4 checksum just
fine.

Note that if ip_summed is CHECKSUM_UNNECESSARY, the padding
bytes (if any)
have no impact on the csum that has been verified by the NIC.

Why? Why does the hardware validates L3/L4 checksum when it
supplies a full-packet checksum? What's its point here?

The point is that the driver author can decide what is best.

For native IP+TCP or IP+UDP, the NIC has the ability to fully
understand the packet and fully validate the checksum.

Also for Native IP4 and IP6 plain L3 packets.
The Hardware validates the csum when it can, and always provides
checksum complete for all packets.
One of the reason to validate is that sometimes we want to skip
checksum complete, but still leverage the hw validation,
like in your patch :), or LRO case, or many other cases in other
kernels/OSes/drivers.

This sounds wrong to me too.

If the HW already validates it, the software doesn't need to do it,
therefore must skip hw csum for performance gain.,

Cong, you are missing some important details, hardware can only parse a
handful of protocols IP/TCP/UDP some tunneling like vxlan,GRE, etc.. 
but for complicated protocols and new generic tunneling protocols,
which the hardware wasn't built to understand, the checksum complete
comes to the rescue.

not only that, imagine you are doing some proprietary tunneling and you
want to encapsulate the rx traffic and send it back to wire, how would
you want to recalculate the csum before txing ? manually on the whole
new packet or use the csum complete and just figure out the differences
? i am sure you gonna want to use the checksum complete of the original
packet.

So again csum complete is not only for validation, it is more powerful
than that.

quoted

So i agree with Eric, let's jump to checksum_unnecessary for short
packets.

This is odd, if Eric is right, then we should completely get rid of
CHECKSUM_COMPLETE. Short packets are not exceptions.

short packets are exception, 
1. because of the small packet padding issue
2. because they are short enough not to feel the performance hit of
recalculating their whole csum (if necessary) .

Again still it is nice to report csum unnecessary for those packets, if
possible.

for large packets csum complete is favorable because of the above
reasons.


if you don't like checksum complete you have a way to totally disable
it in mlx5 via ethtool private flags.

I hope this helps.

I still don't understand why people including Eric kept fixing this
thing which could be just removed from the very beginning.
Sounds like nobody even looked into it until my patch.

Thanks.