| 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
| ||
|
Message-ID: <20250428180349.GF3339421@horms.kernel.org>
Date: Mon, 28 Apr 2025 19:03:49 +0100
From: Simon Horman <horms@...nel.org>
To: Larysa Zaremba <larysa.zaremba@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org,
Tony Nguyen <anthony.l.nguyen@...el.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Jonathan Corbet <corbet@....net>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Jiri Pirko <jiri@...nulli.us>,
Tatyana Nikolova <tatyana.e.nikolova@...el.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
Michael Ellerman <mpe@...erman.id.au>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Lee Trager <lee@...ger.us>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Sridhar Samudrala <sridhar.samudrala@...el.com>,
Jacob Keller <jacob.e.keller@...el.com>,
Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
Mateusz Polchlopek <mateusz.polchlopek@...el.com>,
Ahmed Zaki <ahmed.zaki@...el.com>, netdev@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
"Karlsson, Magnus" <magnus.karlsson@...el.com>,
Emil Tantilov <emil.s.tantilov@...el.com>,
Madhu Chittim <madhu.chittim@...el.com>,
Josh Hay <joshua.a.hay@...el.com>,
Milena Olech <milena.olech@...el.com>, pavan.kumar.linga@...el.com,
"Singhai, Anjali" <anjali.singhai@...el.com>,
Michal Kubiak <michal.kubiak@...el.com>
Subject: Re: [PATCH iwl-next v2 08/14] idpf: refactor idpf to use libie
controlq and Xn APIs
On Thu, Apr 24, 2025 at 01:32:31PM +0200, Larysa Zaremba wrote:
> From: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
>
> Support to initialize and configure controlq, Xn manager,
> MMIO and reset APIs was introduced in libie. As part of it,
> most of the existing controlq structures are renamed and
> modified. Use those APIs in idpf and make all the necessary changes.
>
> Previously for the send and receive virtchnl messages, there
> used to be a memcpy involved in controlq code to copy the buffer
> info passed by the send function into the controlq specific
> buffers. There was no restriction to use automatic memory
> in that case. The new implementation in libie removed copying
> of the send buffer info and introduced DMA mapping of the
> send buffer itself. To accommodate it, use dynamic memory for
> the send buffers. In case of receive, idpf receives a page pool
> buffer allocated by the libie and care should be taken to
> release it after use in the idpf.
>
> The changes are fairly trivial and localized, with a notable exception
> being the consolidation of idpf_vc_xn_shutdown and idpf_deinit_dflt_mbx
> under the latter name. This has some additional consequences that are
> addressed in the following patches.
>
> Reviewed-by: Michal Kubiak <michal.kubiak@...el.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Co-developed-by: Larysa Zaremba <larysa.zaremba@...el.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> ---
> drivers/net/ethernet/intel/idpf/Kconfig | 1 +
> drivers/net/ethernet/intel/idpf/Makefile | 2 -
> drivers/net/ethernet/intel/idpf/idpf.h | 42 +-
> .../net/ethernet/intel/idpf/idpf_controlq.c | 624 -------
> .../net/ethernet/intel/idpf/idpf_controlq.h | 130 --
> .../ethernet/intel/idpf/idpf_controlq_api.h | 177 --
> .../ethernet/intel/idpf/idpf_controlq_setup.c | 171 --
> drivers/net/ethernet/intel/idpf/idpf_dev.c | 91 +-
> drivers/net/ethernet/intel/idpf/idpf_lib.c | 49 +-
> drivers/net/ethernet/intel/idpf/idpf_main.c | 87 +-
> drivers/net/ethernet/intel/idpf/idpf_mem.h | 20 -
> drivers/net/ethernet/intel/idpf/idpf_txrx.h | 2 +-
> drivers/net/ethernet/intel/idpf/idpf_vf_dev.c | 89 +-
> .../net/ethernet/intel/idpf/idpf_virtchnl.c | 1622 ++++++-----------
> .../net/ethernet/intel/idpf/idpf_virtchnl.h | 89 +-
> .../ethernet/intel/idpf/idpf_virtchnl_ptp.c | 303 ++-
> 16 files changed, 886 insertions(+), 2613 deletions(-)
This patch is rather large.
Is there a way it could be split up into more easily reviewable chunks?
> delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.c
> delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h
> delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_api.h
> delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_setup.c
> delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_mem.h
...
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
...
> @@ -2520,15 +2045,18 @@ static void idpf_finalize_ptype_lookup(struct libeth_rx_pt *ptype)
> */
> int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
> {
> - struct virtchnl2_get_ptype_info *get_ptype_info __free(kfree) = NULL;
> - struct virtchnl2_get_ptype_info *ptype_info __free(kfree) = NULL;
> + struct libie_ctlq_xn_send_params xn_params = {
> + .timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> + .chnl_opcode = VIRTCHNL2_OP_GET_PTYPE_INFO,
> + };
> struct libeth_rx_pt *ptype_lkup __free(kfree) = NULL;
> + struct virtchnl2_get_ptype_info *get_ptype_info;
> int max_ptype, ptypes_recvd = 0, ptype_offset;
> struct idpf_adapter *adapter = vport->adapter;
> - struct idpf_vc_xn_params xn_params = {};
> + struct virtchnl2_get_ptype_info *ptype_info;
> + int buf_size = sizeof(*get_ptype_info);
> u16 next_ptype_id = 0;
> - ssize_t reply_sz;
> - int i, j, k;
> + int i, j, k, err = 0;
>
> if (vport->rx_ptype_lkup)
> return 0;
> @@ -2542,22 +2070,11 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
> if (!ptype_lkup)
> return -ENOMEM;
>
> - get_ptype_info = kzalloc(sizeof(*get_ptype_info), GFP_KERNEL);
> - if (!get_ptype_info)
> - return -ENOMEM;
> -
> - ptype_info = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
> - if (!ptype_info)
> - return -ENOMEM;
> -
> - xn_params.vc_op = VIRTCHNL2_OP_GET_PTYPE_INFO;
> - xn_params.send_buf.iov_base = get_ptype_info;
> - xn_params.send_buf.iov_len = sizeof(*get_ptype_info);
> - xn_params.recv_buf.iov_base = ptype_info;
> - xn_params.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN;
> - xn_params.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC;
> -
> while (next_ptype_id < max_ptype) {
> + get_ptype_info = kzalloc(buf_size, GFP_KERNEL);
> + if (!get_ptype_info)
> + return -ENOMEM;
> +
> get_ptype_info->start_ptype_id = cpu_to_le16(next_ptype_id);
>
> if ((next_ptype_id + IDPF_RX_MAX_PTYPES_PER_BUF) > max_ptype)
> @@ -2567,13 +2084,15 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
> get_ptype_info->num_ptypes =
> cpu_to_le16(IDPF_RX_MAX_PTYPES_PER_BUF);
>
> - reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> - if (reply_sz < 0)
> - return reply_sz;
> + err = idpf_send_mb_msg(adapter, &xn_params, get_ptype_info,
> + buf_size);
> + if (err)
> + goto free_tx_buf;
>
> + ptype_info = xn_params.recv_mem.iov_base;
> ptypes_recvd += le16_to_cpu(ptype_info->num_ptypes);
> if (ptypes_recvd > max_ptype)
> - return -EINVAL;
Should err be set to -EINVAL here?
Flagged by Smatch.
> + goto free_rx_buf;
>
> next_ptype_id = le16_to_cpu(get_ptype_info->start_ptype_id) +
> le16_to_cpu(get_ptype_info->num_ptypes);
> @@ -2589,8 +2108,8 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
> ((u8 *)ptype_info + ptype_offset);
>
> ptype_offset += IDPF_GET_PTYPE_SIZE(ptype);
> - if (ptype_offset > IDPF_CTLQ_MAX_BUF_LEN)
> - return -EINVAL;
> + if (ptype_offset > LIBIE_CTLQ_MAX_BUF_LEN)
> + goto free_rx_buf;
>
> /* 0xFFFF indicates end of ptypes */
> if (le16_to_cpu(ptype->ptype_id_10) ==
> @@ -2720,12 +2239,24 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
>
> idpf_finalize_ptype_lookup(&ptype_lkup[k]);
> }
> + libie_ctlq_release_rx_buf(adapter->arq,
> + &xn_params.recv_mem);
> + if (libie_cp_can_send_onstack(buf_size))
> + kfree(get_ptype_info);
> }
>
> out:
> vport->rx_ptype_lkup = no_free_ptr(ptype_lkup);
>
> return 0;
> +
> +free_rx_buf:
> + libie_ctlq_release_rx_buf(adapter->arq, &xn_params.recv_mem);
> +free_tx_buf:
> + if (libie_cp_can_send_onstack(buf_size))
> + kfree(get_ptype_info);
> +
> + return err;
> }
>
> /**
...
Powered by blists - more mailing lists