Re: [PATCH v2] net: multicast: calculate csum of looped-back and forwarded packets

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Sun, Oct 24, 2021 at 4:17 PM Cyril Strejc <cyril.strejc@skoda.cz> wrote:
>
> During a testing of an user-space application which transmits UDP
> multicast datagrams and utilizes multicast routing to send the UDP
> datagrams out of defined network interfaces, I've found a multicast
> router does not fill-in UDP checksum into locally produced, looped-back
> and forwarded UDP datagrams, if an original output NIC the datagrams
> are sent to has UDP TX checksum offload enabled.
>
> The datagrams are sent malformed out of the NIC the datagrams have been
> forwarded to.
>
> It is because:
>
> 1. If TX checksum offload is enabled on the output NIC, UDP checksum
>    is not calculated by kernel and is not filled into skb data.
>
> 2. dev_loopback_xmit(), which is called solely by
>    ip_mc_finish_output(), sets skb->ip_summed = CHECKSUM_UNNECESSARY
>    unconditionally.
>
> 3. Since 35fc92a9 ("[NET]: Allow forwarding of ip_summed except
>    CHECKSUM_COMPLETE"), the ip_summed value is preserved during
>    forwarding.
>
> 4. If ip_summed != CHECKSUM_PARTIAL, checksum is not calculated during
>    a packet egress.
>
> The minimum fix in dev_loopback_xmit():
>
> 1. Preserves skb->ip_summed CHECKSUM_PARTIAL. This is the
>    case when the original output NIC has TX checksum offload enabled.
>    The effects are:
>
>      a) If the forwarding destination interface supports TX checksum
>         offloading, the NIC driver is responsible to fill-in the
>         checksum.
>
>      b) If the forwarding destination interface does NOT support TX
>         checksum offloading, checksums are filled-in by kernel before
>         skb is submitted to the NIC driver.
>
>      c) For local delivery, checksum validation is skipped as in the
>         case of CHECKSUM_UNNECESSARY, thanks to skb_csum_unnecessary().
>
> 2. Translates ip_summed CHECKSUM_NONE to CHECKSUM_UNNECESSARY. It
>    means, for CHECKSUM_NONE, the behavior is unmodified and is there
>    to skip a looped-back packet local delivery checksum validation.
>
> Signed-off-by: Cyril Strejc <cyril.strejc@skoda.cz>

Reviewed-by: Willem de Bruijn <willemb@google.com>