netdev - Re: [Patch net v3] mlx5: force CHECKSUM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 4 Dec 2018 12:31:15 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [Patch net v3] mlx5: force CHECKSUM_NONE for short ethernet frames

On Mon, Dec 3, 2018 at 11:51 PM Eric Dumazet <edumazet@...gle.com> wrote:
> > If it really validates L3/L4 checksum, then a full-packet checksum
> > is not needed.
>
> Yes, this is exactly what CHECKSUM_UNNECESSARY means.
> linux stack does not have to perform the check another time.

So you are suggesting to get rid of CHECKSUM_COMPLETE, right?
Like below:

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 7cb988640c9c..b9626c8b6b91 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -764,32 +764,6 @@ static inline void mlx5e_handle_csum(struct
net_device *netdev,
                return;
        }

-       if (unlikely(test_bit(MLX5E_RQ_STATE_NO_CSUM_COMPLETE, &rq->state)))
-               goto csum_unnecessary;
-
-       if (likely(is_last_ethertype_ip(skb, &network_depth, &proto))) {
-               if (unlikely(get_ip_proto(skb, network_depth, proto)
== IPPROTO_SCTP))
-                       goto csum_unnecessary;
-
-               skb->ip_summed = CHECKSUM_COMPLETE;
-               skb->csum = csum_unfold((__force __sum16)cqe->check_sum);
-               if (network_depth > ETH_HLEN)
-                       /* CQE csum is calculated from the IP header and does
-                        * not cover VLAN headers (if present). This will add
-                        * the checksum manually.
-                        */
-                       skb->csum = csum_partial(skb->data + ETH_HLEN,
-                                                network_depth - ETH_HLEN,
-                                                skb->csum);
-               if (unlikely(netdev->features & NETIF_F_RXFCS))
-                       skb->csum = csum_block_add(skb->csum,
-                                                  (__force
__wsum)mlx5e_get_fcs(skb),
-                                                  skb->len - ETH_FCS_LEN);
-               stats->csum_complete++;
-               return;
-       }
-
-csum_unnecessary:


>
> For example, no call to csum_partial() is needed, even for IPv6+TCP or IPv6+UDP

Sure, if you mean to get rid of CHECKSUM_COMPLETE.

Remember you fixed CHECKSUM_COMPLETE too when you touch it,
see d48051c5b837 ("net/mlx5e: fix csum adjustments caused by RXFCS").
So why didn't you remove CHECKSUM_COMPLETE but fixed it instead?

Thanks.