netdev - Re: [net-next 07/16] net/mlx5: DR, Add support for dumping steering info

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: <20211229181650.33978893@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Wed, 29 Dec 2021 18:16:50 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeed@...nel.org>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Muhammad Sammar <muhammads@...dia.com>,
        Yevgeny Kliteynik <kliteyn@...dia.com>,
        Leon Romanovsky <leonro@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [net-next  07/16] net/mlx5: DR, Add support for dumping
 steering info

On Tue, 28 Dec 2021 22:24:53 -0800 Saeed Mahameed wrote:
> From: Muhammad Sammar <muhammads@...dia.com>
> 
> Extend mlx5 debugfs support to present Software Steering resources:
> dr_domain including it's tables, matchers and rules.
> The interface is read-only. While dump is being presented, new steering
> rules cannot be inserted/deleted.

Looks possibly written against debugfs API as it was a few releases ago?
The return values have changed, see below.

> +static void
> +dr_dump_hex_print(char *dest, u32 dest_size, char *src, u32 src_size)
> +{
> +	int i;
> +
> +	if (dest_size < 2 * src_size)
> +		return;
> +
> +	for (i = 0; i < src_size; i++)
> +		snprintf(&dest[2 * i], BUF_SIZE, "%02x", (u8)src[i]);

bin2hex()

> +}

> +static int
> +dr_dump_rule_mem(struct seq_file *file, struct mlx5dr_ste *ste,
> +		 bool is_rx, const u64 rule_id, u8 format_ver)
> +{
> +	char hw_ste_dump[BUF_SIZE] = {};

seems a little wasteful to zero-init this entire buffer

> +	u32 mem_rec_type;
> +
> +	if (format_ver == MLX5_STEERING_FORMAT_CONNECTX_5) {
> +		mem_rec_type = is_rx ? DR_DUMP_REC_TYPE_RULE_RX_ENTRY_V0 :
> +				       DR_DUMP_REC_TYPE_RULE_TX_ENTRY_V0;
> +	} else {
> +		mem_rec_type = is_rx ? DR_DUMP_REC_TYPE_RULE_RX_ENTRY_V1 :
> +				       DR_DUMP_REC_TYPE_RULE_TX_ENTRY_V1;
> +	}
> +
> +	dr_dump_hex_print(hw_ste_dump, BUF_SIZE, (char *)ste->hw_ste,
> +			  DR_STE_SIZE_REDUCED);
> +
> +	seq_printf(file, "%d,0x%llx,0x%llx,%s\n", mem_rec_type,
> +		   dr_dump_icm_to_idx(mlx5dr_ste_get_icm_addr(ste)), rule_id,
> +		   hw_ste_dump);
> +
> +	return 0;
> +}

> +void mlx5dr_dbg_init_dump(struct mlx5dr_domain *dmn)
> +{
> +	struct mlx5_core_dev *dev = dmn->mdev;
> +	char file_name[128];
> +
> +	if (dmn->type != MLX5DR_DOMAIN_TYPE_FDB) {
> +		mlx5_core_warn(dev,
> +			       "Steering dump is not supported for NIC RX/TX domains\n");
> +		return;
> +	}
> +
> +	if (!dmn->dump_info.steering_debugfs) {
> +		dmn->dump_info.steering_debugfs = debugfs_create_dir("steering",
> +								     dev->priv.dbg_root);
> +		if (!dmn->dump_info.steering_debugfs)

debugfs functions no longer return NULL.

> +			return;
> +	}
> +
> +	if (!dmn->dump_info.fdb_debugfs) {
> +		dmn->dump_info.fdb_debugfs = debugfs_create_dir("fdb",
> +								dmn->dump_info.steering_debugfs);
> +		if (!dmn->dump_info.fdb_debugfs) {

ditto, in fact you're not supposed to check the return values of these
functions at all, they all check if parent is an error pointer an exit
cleanly, so since this is a debug feature just carry on without error
checking

> +			debugfs_remove_recursive(dmn->dump_info.steering_debugfs);
> +			dmn->dump_info.steering_debugfs = NULL;
> +			return;
> +		}
> +	}
> +
> +	sprintf(file_name, "dmn_%p", dmn);
> +	debugfs_create_file(file_name, 0444, dmn->dump_info.fdb_debugfs,
> +			    dmn, &dr_dump_fops);
> +
> +	INIT_LIST_HEAD(&dmn->dbg_tbl_list);
> +	mutex_init(&dmn->dump_info.dbg_mutex);
> +}