On Thu, Jan 31, 2019 at 5:04 AM Tariq Toukan <tar...@mellanox.com> wrote:
>
> From: Saeed Mahameed <sae...@mellanox.com>
>
> When an ethernet frame is padded to meet the minimum ethernet frame
> size, the padding octets are not covered by the hardware checksum.
> Fortunately the padding octets are usually zero's, which don't affect
> checksum. However, it is not guaranteed. For example, switches might
> choose to make other use of these octets.
> This repeatedly causes kernel hardware checksum fault.
>
> Prior to the cited commit below, skb checksum was forced to be
> CHECKSUM_NONE when padding is detected. After it, we need to keep
> skb->csum updated. However, fixing up CHECKSUM_COMPLETE requires to
> verify and parse IP headers, it does not worth the effort as the packets
> are so small that CHECKSUM_COMPLETE has no significant advantage.
>
> Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are
> friends")
> Cc: Eric Dumazet <eduma...@google.com>
> Signed-off-by: Saeed Mahameed <sae...@mellanox.com>
> Signed-off-by: Tariq Toukan <tar...@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> Hi Dave,
> Please queue for -stable >= v4.18.
>
> Thanks,
> Tariq
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 9a0881cb7f51..fc8a11444e1a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -617,6 +617,8 @@ static int get_fixed_ipv6_csum(__wsum hw_checksum, struct
> sk_buff *skb,
> }
> #endif
>
> +#define short_frame(size) ((size) <= ETH_ZLEN + ETH_FCS_LEN)
> +
> /* We reach this function only after checking that any of
> * the (IPv4 | IPv6) bits are set in cqe->status.
> */
> @@ -624,9 +626,22 @@ static int check_csum(struct mlx4_cqe *cqe, struct
> sk_buff *skb, void *va,
> netdev_features_t dev_features)
> {
> __wsum hw_checksum = 0;
> + void *hdr;
> +
> + /* CQE csum doesn't cover padding octets in short ethernet
> + * frames. And the pad field is appended prior to calculating
> + * and appending the FCS field.
> + *
> + * Detecting these padded frames requires to verify and parse
> + * IP headers, so we simply force all those small frames to be
> + * CHECKSUM_NONE even if they are not padded.
> + * TODO: better if we report CHECKSUM_UNNECESSARY but this
> + * demands some refactroing.
This TODO: looks bogus to me.
mlx4 driver first tries to use CHECKSUM_UNNECESSARY.
if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP |
MLX4_CQE_STATUS_UDP)) &&
(cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&
cqe->checksum == cpu_to_be16(0xffff)) {
...
ip_summed = CHECKSUM_UNNECESSARY;
...
}
Then if this attempt failed, it tries to use CHECKSUM_COMPLETE (and
calls this check_sum() function)
So there is no refactoring to be done for mlx4 : short
IPv[46]+{UDP|TCP} frames get CHECKSUM_UNNECESSARY already
> + */
> + if (short_frame(skb->len))
> + return -EINVAL;
>
> - void *hdr = (u8 *)va + sizeof(struct ethhdr);
> -
> + hdr = (u8 *)va + sizeof(struct ethhdr);
> hw_checksum = csum_unfold((__force __sum16)cqe->checksum);
>
> if (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_CVLAN_PRESENT_MASK) &&
> --
> 1.8.3.1
>
