netdev - Re: [PATCH v17 01/20] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZTfSOv0F7licIO6Y@nanopsycho>
Date: Tue, 24 Oct 2023 16:18:34 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Aurelien Aptel <aaptel@...dia.com>
Cc: linux-nvme@...ts.infradead.org, netdev@...r.kernel.org,
	sagi@...mberg.me, hch@....de, kbusch@...nel.org, axboe@...com,
	chaitanyak@...dia.com, davem@...emloft.net, kuba@...nel.org,
	Boris Pismenny <borisp@...dia.com>, aurelien.aptel@...il.com,
	smalin@...dia.com, malin1024@...il.com, ogerlitz@...dia.com,
	yorayz@...dia.com, galshalom@...dia.com, mgurtovoy@...dia.com,
	edumazet@...gle.com, pabeni@...hat.com, dsahern@...nel.org,
	imagedong@...cent.com, ast@...nel.org, jacob.e.keller@...el.com
Subject: Re: [PATCH v17 01/20] net: Introduce direct data placement tcp
 offload

Tue, Oct 24, 2023 at 02:54:26PM CEST, aaptel@...dia.com wrote:
>From: Boris Pismenny <borisp@...dia.com>

[...]


>@@ -2134,6 +2146,9 @@ struct net_device {
> 	netdev_features_t	mpls_features;
> 	netdev_features_t	gso_partial_features;
> 
>+#ifdef CONFIG_ULP_DDP
>+	struct ulp_ddp_netdev_caps ulp_ddp_caps;

Why can't you have this inside the driver? You have set_caps/get_stats
ops. Try to avoid netdev struct pollution.


>+#endif
> 	unsigned int		min_mtu;
> 	unsigned int		max_mtu;
> 	unsigned short		type;

[...]


>+/**
>+ * struct netlink_ulp_ddp_stats - ULP DDP offload statistics
>+ * @rx_nvmeotcp_sk_add: number of sockets successfully prepared for offloading.
>+ * @rx_nvmeotcp_sk_add_fail: number of sockets that failed to be prepared
>+ *                           for offloading.
>+ * @rx_nvmeotcp_sk_del: number of sockets where offloading has been removed.
>+ * @rx_nvmeotcp_ddp_setup: number of NVMeTCP PDU successfully prepared for
>+ *                         Direct Data Placement.
>+ * @rx_nvmeotcp_ddp_setup_fail: number of PDUs that failed DDP preparation.
>+ * @rx_nvmeotcp_ddp_teardown: number of PDUs done with DDP.
>+ * @rx_nvmeotcp_drop: number of PDUs dropped.
>+ * @rx_nvmeotcp_resync: number of resync.
>+ * @rx_nvmeotcp_packets: number of offloaded PDUs.
>+ * @rx_nvmeotcp_bytes: number of offloaded bytes.
>+ */
>+struct netlink_ulp_ddp_stats {

There is nothing "netlink" about this. Just stats. Exposed over netlink,
yes, but that does not need the prefix.


>+	u64 rx_nvmeotcp_sk_add;
>+	u64 rx_nvmeotcp_sk_add_fail;
>+	u64 rx_nvmeotcp_sk_del;
>+	u64 rx_nvmeotcp_ddp_setup;
>+	u64 rx_nvmeotcp_ddp_setup_fail;
>+	u64 rx_nvmeotcp_ddp_teardown;
>+	u64 rx_nvmeotcp_drop;
>+	u64 rx_nvmeotcp_resync;
>+	u64 rx_nvmeotcp_packets;
>+	u64 rx_nvmeotcp_bytes;
>+
>+	/*
>+	 * add new stats at the end and keep in sync with
>+	 * Documentation/netlink/specs/ulp_ddp.yaml
>+	 */
>+};

[...]


>+++ b/include/net/ulp_ddp_caps.h
>@@ -0,0 +1,42 @@
>+/* SPDX-License-Identifier: GPL-2.0
>+ *
>+ * ulp_ddp.h
>+ *  Author: Aurelien Aptel <aaptel@...dia.com>
>+ *  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES.  All rights reserved.
>+ */
>+#ifndef _ULP_DDP_CAPS_H
>+#define _ULP_DDP_CAPS_H
>+
>+#include <linux/types.h>
>+
>+enum {
>+	ULP_DDP_C_NVME_TCP_BIT,
>+	ULP_DDP_C_NVME_TCP_DDGST_RX_BIT,
>+
>+	/*
>+	 * add capabilities above and keep in sync with
>+	 * Documentation/netlink/specs/ulp_ddp.yaml

Wait what? Why do you need this at all? Just use the uapi enum.


>+	 */
>+	ULP_DDP_C_COUNT,
>+};
>+

[...]