netdev - Re: [net-next V2 12/12] net/mlx5e: Use PARTIAL

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] [day] [month] [year] [list]

Date:   Wed, 1 Aug 2018 04:32:50 -0400
From:   Boris Pismenny <borisp@...lanox.com>
To:     Saeed Mahameed <saeedm@....mellanox.co.il>,
        Alexander Duyck <alexander.duyck@...il.com>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [net-next V2 12/12] net/mlx5e: Use PARTIAL_GSO for UDP
 segmentation



On 7/24/2018 8:35 PM, Saeed Mahameed wrote:
> On Tue, Jul 24, 2018 at 7:53 AM, Alexander Duyck
> <alexander.duyck@...il.com> wrote:
>> On Mon, Jul 23, 2018 at 3:11 PM, Saeed Mahameed <saeedm@...lanox.com> wrote:
>>> From: Boris Pismenny <borisp@...lanox.com>
>>>
>>> This patch removes the splitting of UDP_GSO_L4 packets in the driver,
>>> and exposes UDP_GSO_L4 as a PARTIAL_GSO feature. Thus, the network stack
>>> is not responsible for splitting the packet into two.
>>>
>>> Signed-off-by: Boris Pismenny <borisp@...lanox.com>
>>> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
>>> ---
>>>   .../net/ethernet/mellanox/mlx5/core/Makefile  |   4 +-
>>>   .../mellanox/mlx5/core/en_accel/en_accel.h    |  27 +++--
>>>   .../mellanox/mlx5/core/en_accel/rxtx.c        | 109 ------------------
>>>   .../mellanox/mlx5/core/en_accel/rxtx.h        |  14 ---
>>>   .../net/ethernet/mellanox/mlx5/core/en_main.c |   9 +-
>>>   5 files changed, 23 insertions(+), 140 deletions(-)
>>>   delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
>>>   delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> index 55d5a5c2e9d8..fa7fcca5dc78 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> @@ -14,8 +14,8 @@ mlx5_core-$(CONFIG_MLX5_FPGA) += fpga/cmd.o fpga/core.o fpga/conn.o fpga/sdk.o \
>>>                  fpga/ipsec.o fpga/tls.o
>>>
>>>   mlx5_core-$(CONFIG_MLX5_CORE_EN) += en_main.o en_common.o en_fs.o en_ethtool.o \
>>> -               en_tx.o en_rx.o en_dim.o en_txrx.o en_accel/rxtx.o en_stats.o  \
>>> -               vxlan.o en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
>>> +               en_tx.o en_rx.o en_dim.o en_txrx.o en_stats.o vxlan.o          \
>>> +               en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
>>>
>>>   mlx5_core-$(CONFIG_MLX5_MPFS) += lib/mpfs.o
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> index 39a5d13ba459..1dd225380a66 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> @@ -38,14 +38,22 @@
>>>   #include <linux/netdevice.h>
>>>   #include "en_accel/ipsec_rxtx.h"
>>>   #include "en_accel/tls_rxtx.h"
>>> -#include "en_accel/rxtx.h"
>>>   #include "en.h"
>>>
>>> -static inline struct sk_buff *mlx5e_accel_handle_tx(struct sk_buff *skb,
>>> -                                                   struct mlx5e_txqsq *sq,
>>> -                                                   struct net_device *dev,
>>> -                                                   struct mlx5e_tx_wqe **wqe,
>>> -                                                   u16 *pi)
>>> +static inline void
>>> +mlx5e_udp_gso_handle_tx_skb(struct sk_buff *skb)
>>> +{
>>> +       int payload_len = skb_shinfo(skb)->gso_size + sizeof(struct udphdr);
>>> +
>>> +       udp_hdr(skb)->len = htons(payload_len);
>>> +}
>>> +
>>
>> So it looks like you decided to just update the length here. Do you
>> still have plans to update GSO_PARTIAL to set the length this way or
>> have you decided to just leave it as it is?
>>
> 
> I don't know what are Boris's plans regarding this, I will let him answer that.
> But what is your take on this ?
> 

Sorry for the late response. I'm currently busy with other things. I'd 
rather let it be like this for now, and fix it later. If you'd like to 
change it, I'll be happy to test.

Thanks