netdev - Re: [PATCH net-next v2 3/4] ethtool: Add support for configuring tcp-data-split-thresh

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMArcTUXpDHW2+67gT5tUh9Sw-02zVMGWD85fjmuJ2vy431Ydw@mail.gmail.com>
Date: Thu, 12 Sep 2024 00:42:42 +0900
From: Taehee Yoo <ap420073@...il.com>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	edumazet@...gle.com, corbet@....net, michael.chan@...adcom.com, 
	netdev@...r.kernel.org, linux-doc@...r.kernel.org, ecree.xilinx@...il.com, 
	przemyslaw.kitszel@...el.com, andrew@...n.ch, hkallweit1@...il.com, 
	ahmed.zaki@...el.com, paul.greenwalt@...el.com, rrameshbabu@...dia.com, 
	idosch@...dia.com, maxime.chevallier@...tlin.com, danieller@...dia.com, 
	aleksander.lobakin@...el.com
Subject: Re: [PATCH net-next v2 3/4] ethtool: Add support for configuring tcp-data-split-thresh

On Thu, Sep 12, 2024 at 12:26 AM Kory Maincent
<kory.maincent@...tlin.com> wrote:
>

Hi Kory, Thank you so much for the review!

> On Wed, 11 Sep 2024 14:55:54 +0000
> Taehee Yoo <ap420073@...il.com> wrote:
>
> > The tcp-data-split-thresh option configures the threshold value of
> > the tcp-data-split.
> > If a received packet size is larger than this threshold value, a packet
> > will be split into header and payload.
> > The header indicates TCP header, but it depends on driver spec.
> > The bnxt_en driver supports HDS(Header-Data-Split) configuration at
> > FW level, affecting TCP and UDP too.
> > So, like the tcp-data-split option, If tcp-data-split-thresh is set,
> > it affects UDP and TCP packets.
>
> Could you add a patch to modify the specs accordingly?
> The specs are located here: Documentation/netlink/specs/ethtool.yaml
> You can use ./tools/net/ynl tool and these specs to test ethtool netlink
> messages.
>
> Use this to verify that your specs update are well written.
> $ make -C tools/net/ynl

Thanks a lot! I will add a patch for ethtool.yaml.

>
> > diff --git a/Documentation/networking/ethtool-netlink.rst
> > b/Documentation/networking/ethtool-netlink.rst index
> > ba90457b8b2d..bb74e108c8c1 100644 ---
> > a/Documentation/networking/ethtool-netlink.rst +++
> > b/Documentation/networking/ethtool-netlink.rst @@ -892,6 +892,7 @@ Kernel
> > response contents: ``ETHTOOL_A_RINGS_RX_PUSH``               u8      flag of
> > RX Push mode ``ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN``       u32     size of TX
> > push buffer ``ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN_MAX``   u32     max size of TX
> > push buffer
> > +  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT_THRESH`` u32     threshold of TDS
> >    =======================================   ======
> > ===========================
>
> It seems there is a misalignment here. You need two more '=='
>
> >  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT`` indicates whether the device is usable
> > with @@ -927,18 +928,20 @@ Sets ring sizes like ``ETHTOOL_SRINGPARAM`` ioctl
> > request.
> >  Request contents:
> >
> > -  ====================================  ======  ===========================
> > -  ``ETHTOOL_A_RINGS_HEADER``            nested  reply header
> > -  ``ETHTOOL_A_RINGS_RX``                u32     size of RX ring
> > -  ``ETHTOOL_A_RINGS_RX_MINI``           u32     size of RX mini ring
> > -  ``ETHTOOL_A_RINGS_RX_JUMBO``          u32     size of RX jumbo ring
> > -  ``ETHTOOL_A_RINGS_TX``                u32     size of TX ring
> > -  ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
> > -  ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
> > -  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
> > -  ``ETHTOOL_A_RINGS_RX_PUSH``           u8      flag of RX Push mode
> > -  ``ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN``   u32     size of TX push buffer
> > -  ====================================  ======  ===========================
> > +  =======================================   ======
> > ===========================
> > +  ``ETHTOOL_A_RINGS_HEADER``                nested  reply header
> > +  ``ETHTOOL_A_RINGS_RX``                    u32     size of RX ring
> > +  ``ETHTOOL_A_RINGS_RX_MINI``               u32     size of RX mini ring
> > +  ``ETHTOOL_A_RINGS_RX_JUMBO``              u32     size of RX jumbo ring
> > +  ``ETHTOOL_A_RINGS_TX``                    u32     size of TX ring
> > +  ``ETHTOOL_A_RINGS_RX_BUF_LEN``            u32     size of buffers on the
> > ring
> > +  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT``        u8      TCP header / data split
> > +  ``ETHTOOL_A_RINGS_CQE_SIZE``              u32     Size of TX/RX CQE
> > +  ``ETHTOOL_A_RINGS_TX_PUSH``               u8      flag of TX Push mode
> > +  ``ETHTOOL_A_RINGS_RX_PUSH``               u8      flag of RX Push mode
> > +  ``ETHTOOL_A_RINGS_TX_PUSH_BUF_LEN``       u32     size of TX push buffer
> > +  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT_THRESH`` u32     threshold of TDS
> > +  =======================================   ======
> > ===========================
>
> same here.

Thanks, I will fix this too.

>
> --
> Köry Maincent, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com

Thanks a lot!
Taehee Yoo