netdev - RE: [PATCH v7 01/23] net: Introduce direct data placement tcp 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: <253k04ct08y.fsf@mtr-vdi-124.i-did-not-set--mail-host-address--so-tickle-me>
Date:   Thu, 03 Nov 2022 19:29:33 +0200
From:   Aurelien Aptel <aaptel@...dia.com>
To:     Shai Malin <smalin@...dia.com>, Jakub Kicinski <kuba@...nel.org>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Tariq Toukan <tariqt@...dia.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "sagi@...mberg.me" <sagi@...mberg.me>, "hch@....de" <hch@....de>,
        "kbusch@...nel.org" <kbusch@...nel.org>,
        "axboe@...com" <axboe@...com>,
        Chaitanya Kulkarni <chaitanyak@...dia.com>,
        Or Gerlitz <ogerlitz@...dia.com>,
        Yoray Zack <yorayz@...dia.com>,
        Boris Pismenny <borisp@...dia.com>,
        "aurelien.aptel@...il.com" <aurelien.aptel@...il.com>,
        "malin1024@...il.com" <malin1024@...il.com>
Subject: RE: [PATCH v7 01/23] net: Introduce direct data placement tcp offload

Jakub,

We came up with 2 designs for controlling the ULP DDP capability bits
and getting the ULP DDP statistics.

Both designs share some concepts so I'm going to talk about the common
stuff first:

We drop the netdev->feature bit. To fully disable ULP DDP offload the
caps will have to be set to 0x0.

In both design we replace the feature bit with a new field
netdev->ulp_ddp_caps

struct ulp_ddp_cap {
        bitmap caps_hw;     // what the hw supports (filled by the driver, used as reference once initialized)
        bitmap caps_active; // what is currently set for the system, can be modified from userspace
};

We add a new OP net_device_ops->ndo_set_ulp_caps() that drivers have
to provide to fill netdev->ulp_ddp_caps.caps_hw.  We call it around
the same time as when we call ndo_set_features().

Interfacing with userspace is where the design differs.

Design A ("netlink"):
=====================

# Capabilities

We can expose to the users a new ethtool api using netlink.

For this we want to have a dynamic system where userspace doesn't have
to hardcode all the caps but instead can get a list.  We implement
something similar to what is done for features bits.

We add a table to map caps to string names

const char *ulp_ddp_cap_names[] = {
        [ULP_DDP_NVME_TCP_XXX] = "nvme-tcp-xxx",
        ...
};

We add ETHTOOL messages to get and set ULP caps:

- ETHTOOL_MSG_ULP_CAPS_GET: get device ulp capabilities
- ETHTOOL_MSG_ULP_CAPS_SET: set device up capabilities

The GET reply code can use ethnl_put_bitset32() which does the job of
sending bits + their names as strings.

The SET code would apply the changes to netdev->ulp_ddp_caps.caps_active.

# Statistics

If the ETHTOOL_MSG_ULP_CAPS_GET message requests statistics (by
setting the header flag ETHTOOL_FLAG_STATS) the kernel will append all
the ULP statistics of the device at the end of the reply.

Those statistics will be dynamic in the sense that they will use a new
stringset for their names that userspace will have to fetch.

# Ethtool changes
We will add the -u|-U|--ulp-get|--ulp-set options to ethtool.

   # query list of caps supported and their value on device $dev
   ethtool -u|--ulp-get <dev>

   # query ULP stats of $dev
   ethtool -u|--ulp-get --include-statistics <dev>

   # set $cap to $val on device $dev
   -U|--ulp-set <dev> <cap> [on|off]

Design B ("procfs")
===================

In this design we add a new /proc/sys/net/ulp/* hierarchy, under which
we will add a directory per device (e.g. /proc/sys/net/ulp/eth0/) to
configure/query ULP DDP.

# Capabilities

    # set capabilities per device
    $ echo 0x1 > /proc/sys/net/ulp/<device>/caps

# Statistics

    # show per device stats (global and per queue)
    # space separated values, 1 stat per line
    $ cat /proc/sys/net/ulp/<device>/stats
    rx_nvmeotcp_drop 0
    rx_nvmeotcp_resync 403
    rx_nvmeotcp_offload_packets 75614185
    rx_nvmeotcp_offload_bytes 107016641528
    rx_nvmeotcp_sk_add 1
    rx_nvmeotcp_sk_add_fail 0
    rx_nvmeotcp_sk_del 0
    rx_nvmeotcp_ddp_setup 3327969
    rx_nvmeotcp_ddp_setup_fail 0
    rx_nvmeotcp_ddp_teardown 3327969

I can also suggest the existing paths:

- /sys/class/net/<device>/statistics/
- /proc/net/stat/

Or any other path you will prefer.

We will appreciate your feedback.
Thanks