netdev - Re: [PATCH RFC 3/3] mlx4: Call skb_csum_offload

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]

Message-ID: <CALx6S35XHzp=1b=XpiG2Vrc4ZRUiMyfbLCb5Q840ZWuK-PRVyQ@mail.gmail.com>
Date:	Tue, 6 Oct 2015 09:22:27 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Kernel Team <kernel-team@...com>,
	David Woodhouse <dwmw2@...radead.org>,
	Or Gerlitz <ogerlitz@...lanox.com>
Subject: Re: [PATCH RFC 3/3] mlx4: Call skb_csum_offload_check to check offloadability

On Mon, Oct 5, 2015 at 9:03 PM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On 10/05/2015 04:39 PM, Tom Herbert wrote:
>>
>> This provides an example of a driver calling the skb_csum_offload_check.
>>
>> Signed-off-by: Tom Herbert <tom@...bertland.com>
>> ---
>>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  6 +++---
>>   drivers/net/ethernet/mellanox/mlx4/en_tx.c     | 20 +++++++++++++++-----
>>   2 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 4726122..f2ed8d0 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -2360,7 +2360,7 @@ out:
>>         }
>>         /* set offloads */
>> -       priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>> +       priv->dev->hw_enc_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
>>                                       NETIF_F_TSO |
>> NETIF_F_GSO_UDP_TUNNEL;
>>         priv->dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
>>         priv->dev->features    |= NETIF_F_GSO_UDP_TUNNEL;
>> @@ -2372,7 +2372,7 @@ static void mlx4_en_del_vxlan_offloads(struct
>> work_struct *work)
>>         struct mlx4_en_priv *priv = container_of(work, struct
>> mlx4_en_priv,
>>                                                  vxlan_del_task);
>>         /* unset offloads */
>> -       priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>> +       priv->dev->hw_enc_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
>>                                       NETIF_F_TSO |
>> NETIF_F_GSO_UDP_TUNNEL);
>>         priv->dev->hw_features &= ~NETIF_F_GSO_UDP_TUNNEL;
>>         priv->dev->features    &= ~NETIF_F_GSO_UDP_TUNNEL;
>> @@ -2943,7 +2943,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev,
>> int port,
>>         /*
>>          * Set driver features
>>          */
>> -       dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM |
>> NETIF_F_IPV6_CSUM;
>> +       dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM;
>>         if (mdev->LSO_support)
>>                 dev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
>>   diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> index 494e776..f364ffd 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> @@ -702,6 +702,14 @@ static void mlx4_bf_copy(void __iomem *dst, const
>> void *src,
>>         __iowrite64_copy(dst, src, bytecnt / 8);
>>   }
>>   +static const struct skb_csum_offl_spec csum_offl_spec = {
>> +       .ipv4_okay = 1,
>> +       .ipv6_okay = 1,
>> +       .encap_okay = 1,
>> +       .tcp_okay = 1,
>> +       .udp_okay = 1,
>> +};
>> +
>
>
> The question I would have is if inner IPv6 checksum is supported by this
> driver.  The code before didn't seem to indicate it was, and after the
> csum_offl_spec would seem to indicate it is.  One of my concerns about a
> change like this is that it is likely prone to introduce regressions as
> features are going to be toggling due to interpretations of flags and
> assumptions about what is good for the outer headers is good for the inner
> ones.

Do you mean to say that there could be a device that supports an inner
and outer checksum for IPv4, but only an outer checksum for IPv6 and
not inner checksum?

Tom
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html