Message ID | 1549901057-2614-1-git-send-email-tariqt@mellanox.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net,V2] net/mlx4_en: Force CHECKSUM_NONE for short ethernet frames | expand |
On 02/11/2019 08:04 AM, Tariq Toukan wrote: > From: Saeed Mahameed <saeedm@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. > > Future work: when reporting checksum complete is not an option for > IP non-TCP/UDP packets, we can actually fallback to report checksum > unnecessary, by looking at cqe IPOK bit. > > Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends") > Cc: Eric Dumazet <edumazet@google.com> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com> Thanks Tariq and Saeed Reviewed-by: Eric Dumazet <edumazet@google.com>
From: Tariq Toukan <tariqt@mellanox.com> Date: Mon, 11 Feb 2019 18:04:17 +0200 > From: Saeed Mahameed <saeedm@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. > > Future work: when reporting checksum complete is not an option for > IP non-TCP/UDP packets, we can actually fallback to report checksum > unnecessary, by looking at cqe IPOK bit. > > Fixes: 88078d98d1bb ("net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends") > Cc: Eric Dumazet <edumazet@google.com> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com> Applied and queued up for -stable.
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c index 9a0881cb7f51..6c01314e87b0 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,20 @@ 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 skip + * checksum complete. + */ + 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) && @@ -819,6 +832,11 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud skb_record_rx_queue(skb, cq_ring); if (likely(dev->features & NETIF_F_RXCSUM)) { + /* TODO: For IP non TCP/UDP packets when csum complete is + * not an option (not supported or any other reason) we can + * actually check cqe IPOK status bit and report + * CHECKSUM_UNNECESSARY rather than CHECKSUM_NONE + */ if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | MLX4_CQE_STATUS_UDP)) && (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&