| 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
| ||
|
Date: Thu, 13 Jun 2024 15:01:09 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Omer Shpigelman <oshpigelman@...ana.ai>, <linux-kernel@...r.kernel.org>,
<linux-rdma@...r.kernel.org>, <netdev@...r.kernel.org>,
<dri-devel@...ts.freedesktop.org>
CC: <ogabbay@...nel.org>, <zyehudai@...ana.ai>
Subject: Re: [PATCH 01/15] net: hbl_cn: add habanalabs Core Network driver
On 6/13/24 10:21, Omer Shpigelman wrote:
> Add the hbl_cn driver which will serve both Ethernet and InfiniBand
> drivers.
> hbl_cn is the layer which is used by the satellite drivers for many shared
> operations that are needed by both EN and IB subsystems like QPs, CQs etc.
> The CN driver is initialized via auxiliary bus by the habanalabs driver.
>
> Signed-off-by: Omer Shpigelman <oshpigelman@...ana.ai>
> Co-developed-by: Abhilash K V <kvabhilash@...ana.ai>
> Signed-off-by: Abhilash K V <kvabhilash@...ana.ai>
> Co-developed-by: Andrey Agranovich <aagranovich@...ana.ai>
> Signed-off-by: Andrey Agranovich <aagranovich@...ana.ai>
> Co-developed-by: Bharat Jauhari <bjauhari@...ana.ai>
> Signed-off-by: Bharat Jauhari <bjauhari@...ana.ai>
> Co-developed-by: David Meriin <dmeriin@...ana.ai>
> Signed-off-by: David Meriin <dmeriin@...ana.ai>
> Co-developed-by: Sagiv Ozeri <sozeri@...ana.ai>
> Signed-off-by: Sagiv Ozeri <sozeri@...ana.ai>
> Co-developed-by: Zvika Yehudai <zyehudai@...ana.ai>
> Signed-off-by: Zvika Yehudai <zyehudai@...ana.ai>
> ---
> .../device_drivers/ethernet/index.rst | 1 +
> .../device_drivers/ethernet/intel/hbl.rst | 82 +
> MAINTAINERS | 11 +
> drivers/net/ethernet/intel/Kconfig | 20 +
> drivers/net/ethernet/intel/Makefile | 1 +
> drivers/net/ethernet/intel/hbl_cn/Makefile | 9 +
> .../net/ethernet/intel/hbl_cn/common/Makefile | 3 +
> .../net/ethernet/intel/hbl_cn/common/hbl_cn.c | 5954 +++++++++++++++++
> .../net/ethernet/intel/hbl_cn/common/hbl_cn.h | 1627 +++++
> .../ethernet/intel/hbl_cn/common/hbl_cn_drv.c | 220 +
> .../intel/hbl_cn/common/hbl_cn_memory.c | 40 +
> .../ethernet/intel/hbl_cn/common/hbl_cn_phy.c | 33 +
> .../ethernet/intel/hbl_cn/common/hbl_cn_qp.c | 13 +
> include/linux/habanalabs/cpucp_if.h | 125 +-
> include/linux/habanalabs/hl_boot_if.h | 9 +-
> include/linux/net/intel/cn.h | 474 ++
> include/linux/net/intel/cn_aux.h | 298 +
> include/linux/net/intel/cni.h | 636 ++
> 18 files changed, 9545 insertions(+), 11 deletions(-)
this is a very big patch, it asks for a split; what's worse, it's
proportional to the size of this series:
146 files changed, 148514 insertions(+), 70 deletions(-)
which is just too big
[...]
> +Support
> +=======
> +For general information, go to the Intel support website at:
> +https://www.intel.com/support/
> +
> +If an issue is identified with the released source code on a supported kernel
> +with a supported adapter, email the specific information related to the issue
> +to intel-wired-lan@...ts.osuosl.org.
I'm welcoming you to post next version of the driver to the IWL mailing
list, and before that, to go through our Intel path for ethernet
subsystem (rdma and a few smaller ones also go through that)
(that starts internally, I will PM you the details)
[...]
> +++ b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn.c
> @@ -0,0 +1,5954 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2020-2024 HabanaLabs, Ltd.
> + * Copyright (C) 2023-2024, Intel Corporation.
> + * All Rights Reserved.
> + */
> +
> +#include "hbl_cn.h"
> +
> +#include <linux/file.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +
> +#define NIC_MIN_WQS_PER_PORT 2
> +
> +#define NIC_SEQ_RESETS_TIMEOUT_MS 15000 /* 15 seconds */
> +#define NIC_MAX_SEQ_RESETS 3
> +
> +#define HBL_CN_IPV4_PROTOCOL_UDP 17
> +
> +/* SOB mask is not expected to change across ASIC. Hence common defines. */
> +#define NIC_SOB_INC_MASK 0x80000000
> +#define NIC_SOB_VAL_MASK 0x7fff
> +
> +#define NIC_DUMP_QP_SZ SZ_4K
> +
> +#define HBL_AUX2NIC(aux_dev) \
> + ({ \
> + struct hbl_aux_dev *__aux_dev = (aux_dev); \
> + ((__aux_dev)->type == HBL_AUX_DEV_ETH) ? \
> + container_of(__aux_dev, struct hbl_cn_device, en_aux_dev) : \
> + container_of(__aux_dev, struct hbl_cn_device, ib_aux_dev); \
> + })
this should be a function
> +
> +#define RAND_STAT_CNT(cnt) \
> + do { \
> + u32 __cnt = get_random_u32(); \
> + (cnt) = __cnt; \
> + dev_info(hdev->dev, "port %d, %s: %u\n", port, #cnt, __cnt); \
no way for such message, ditto for the function
> + } while (0)
> +
> +struct hbl_cn_stat hbl_cn_mac_fec_stats[] = {
> + {"correctable_errors", 0x2, 0x3},
> + {"uncorrectable_errors", 0x4, 0x5}
> +};
> +
> +struct hbl_cn_stat hbl_cn_mac_stats_rx[] = {
> + {"Octets", 0x0},
> + {"OctetsReceivedOK", 0x4},
> + {"aAlignmentErrors", 0x8},
> + {"aPAUSEMACCtrlFramesReceived", 0xC},
> + {"aFrameTooLongErrors", 0x10},
> + {"aInRangeLengthErrors", 0x14},
> + {"aFramesReceivedOK", 0x18},
> + {"aFrameCheckSequenceErrors", 0x1C},
> + {"VLANReceivedOK", 0x20},
> + {"ifInErrors", 0x24},
> + {"ifInUcastPkts", 0x28},
> + {"ifInMulticastPkts", 0x2C},
> + {"ifInBroadcastPkts", 0x30},
> + {"DropEvents", 0x34},
> + {"Pkts", 0x38},
> + {"UndersizePkts", 0x3C},
> + {"Pkts64Octets", 0x40},
> + {"Pkts65to127Octets", 0x44},
> + {"Pkts128to255Octets", 0x48},
> + {"Pkts256to511Octets", 0x4C},
> + {"Pkts512to1023Octets", 0x50},
> + {"Pkts1024to1518Octets", 0x54},
> + {"Pkts1519toMaxOctets", 0x58},
> + {"OversizePkts", 0x5C},
> + {"Jabbers", 0x60},
> + {"Fragments", 0x64},
> + {"aCBFCPAUSERx0", 0x68},
> + {"aCBFCPAUSERx1", 0x6C},
> + {"aCBFCPAUSERx2", 0x70},
> + {"aCBFCPAUSERx3", 0x74},
> + {"aCBFCPAUSERx4", 0x78},
> + {"aCBFCPAUSERx5", 0x7C},
> + {"aCBFCPAUSERx6", 0x80},
> + {"aCBFCPAUSERx7", 0x84},
> + {"aMACControlFramesReceived", 0x88}
> +};
> +
> +struct hbl_cn_stat hbl_cn_mac_stats_tx[] = {
> + {"Octets", 0x0},
> + {"OctetsTransmittedOK", 0x4},
> + {"aPAUSEMACCtrlFramesTransmitted", 0x8},
> + {"aFramesTransmittedOK", 0xC},
> + {"VLANTransmittedOK", 0x10},
> + {"ifOutErrors", 0x14},
> + {"ifOutUcastPkts", 0x18},
> + {"ifOutMulticastPkts", 0x1C},
> + {"ifOutBroadcastPkts", 0x20},
> + {"Pkts64Octets", 0x24},
> + {"Pkts65to127Octets", 0x28},
> + {"Pkts128to255Octets", 0x2C},
> + {"Pkts256to511Octets", 0x30},
> + {"Pkts512to1023Octets", 0x34},
> + {"Pkts1024to1518Octets", 0x38},
> + {"Pkts1519toMaxOctets", 0x3C},
> + {"aCBFCPAUSETx0", 0x40},
> + {"aCBFCPAUSETx1", 0x44},
> + {"aCBFCPAUSETx2", 0x48},
> + {"aCBFCPAUSETx3", 0x4C},
> + {"aCBFCPAUSETx4", 0x50},
> + {"aCBFCPAUSETx5", 0x54},
> + {"aCBFCPAUSETx6", 0x58},
> + {"aCBFCPAUSETx7", 0x5C},
> + {"aMACControlFramesTx", 0x60},
> + {"Pkts", 0x64}
> +};
> +
> +static const char pcs_counters_str[][ETH_GSTRING_LEN] = {
> + {"pcs_local_faults"},
> + {"pcs_remote_faults"},
> + {"pcs_remote_fault_reconfig"},
> + {"pcs_link_restores"},
> + {"pcs_link_toggles"},
> +};
> +
> +static size_t pcs_counters_str_len = ARRAY_SIZE(pcs_counters_str);
> +size_t hbl_cn_mac_fec_stats_len = ARRAY_SIZE(hbl_cn_mac_fec_stats);
> +size_t hbl_cn_mac_stats_rx_len = ARRAY_SIZE(hbl_cn_mac_stats_rx);
> +size_t hbl_cn_mac_stats_tx_len = ARRAY_SIZE(hbl_cn_mac_stats_tx);
why those are not const?
> +
> +static void qps_stop(struct hbl_cn_device *hdev);
> +static void qp_destroy_work(struct work_struct *work);
> +static int __user_wq_arr_unset(struct hbl_cn_ctx *ctx, struct hbl_cn_port *cn_port, u32 type);
> +static void user_cq_destroy(struct kref *kref);
> +static void set_app_params_clear(struct hbl_cn_device *hdev);
> +static int hbl_cn_ib_cmd_ctrl(struct hbl_aux_dev *aux_dev, void *cn_ib_ctx, u32 op, void *input,
> + void *output);
> +static int hbl_cn_ib_query_mem_handle(struct hbl_aux_dev *ib_aux_dev, u64 mem_handle,
> + struct hbl_ib_mem_info *info);
> +
> +static void hbl_cn_reset_stats_counters_port(struct hbl_cn_device *hdev, u32 port);
> +static void hbl_cn_late_init(struct hbl_cn_device *hdev);
> +static void hbl_cn_late_fini(struct hbl_cn_device *hdev);
> +static int hbl_cn_sw_init(struct hbl_cn_device *hdev);
> +static void hbl_cn_sw_fini(struct hbl_cn_device *hdev);
> +static void hbl_cn_spmu_init(struct hbl_cn_port *cn_port, bool full);
> +static int hbl_cn_cmd_port_check(struct hbl_cn_device *hdev, u32 port, u32 flags);
> +static void hbl_cn_qps_stop(struct hbl_cn_port *cn_port);
> +
> +static int hbl_cn_request_irqs(struct hbl_cn_device *hdev)
> +{
> + struct hbl_cn_asic_funcs *asic_funcs = hdev->asic_funcs;
> +
> + return asic_funcs->request_irqs(hdev);
> +}
> +
> +static void hbl_cn_free_irqs(struct hbl_cn_device *hdev)
> +{
> + struct hbl_cn_asic_funcs *asic_funcs = hdev->asic_funcs;
> +
> + asic_funcs->free_irqs(hdev);
> +}
> +
> +static void hbl_cn_synchronize_irqs(struct hbl_aux_dev *cn_aux_dev)
> +{
> + struct hbl_cn_device *hdev = cn_aux_dev->priv;
> + struct hbl_cn_asic_funcs *asic_funcs;
> +
> + asic_funcs = hdev->asic_funcs;
> +
> + asic_funcs->synchronize_irqs(hdev);
> +}
> +
> +void hbl_cn_get_frac_info(u64 numerator, u64 denominator, u64 *integer, u64 *exp)
> +{
> + u64 high_digit_n, high_digit_d, integer_tmp, exp_tmp;
> + u8 num_digits_n, num_digits_d;
> + int i;
> +
> + num_digits_d = hbl_cn_get_num_of_digits(denominator);
> + high_digit_d = denominator;
> + for (i = 0; i < num_digits_d - 1; i++)
> + high_digit_d /= 10;
> +
> + integer_tmp = 0;
> + exp_tmp = 0;
> +
> + if (numerator) {
> + num_digits_n = hbl_cn_get_num_of_digits(numerator);
> + high_digit_n = numerator;
> + for (i = 0; i < num_digits_n - 1; i++)
> + high_digit_n /= 10;
> +
> + exp_tmp = num_digits_d - num_digits_n;
> +
> + if (high_digit_n < high_digit_d) {
> + high_digit_n *= 10;
> + exp_tmp++;
> + }
> +
> + integer_tmp = div_u64(high_digit_n, high_digit_d);
> + }
> +
> + *integer = integer_tmp;
> + *exp = exp_tmp;
> +}
this function sounds suspicious for a network driver, what do you need
it for?
> +
> +int hbl_cn_read_spmu_counters(struct hbl_cn_port *cn_port, u64 out_data[], u32 *num_out_data)
> +{
> + struct hbl_cn_device *hdev = cn_port->hdev;
> + struct hbl_cn_asic_port_funcs *port_funcs;
> + struct hbl_cn_stat *ignore;
> + int rc;
> +
> + port_funcs = hdev->asic_funcs->port_funcs;
> +
> + port_funcs->spmu_get_stats_info(cn_port, &ignore, num_out_data);
hard to ignore that you deref uninitialized pointer...
please consider going one step back and start with our internal mailing
lists, thank you
Przemek
[...]
Powered by blists - more mailing lists