netdev - Re: [PATCH net-next 1/3] qed: Add DCBX API

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] [day] [month] [year] [list]

Date:   Tue, 31 Jul 2018 09:58:20 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     denis.bolotin@...ium.com
Cc:     netdev@...r.kernel.org, ariel.elior@...ium.com
Subject: Re: [PATCH net-next 1/3] qed: Add DCBX API -
 qed_dcbx_get_priority_tc()

From: Denis Bolotin <denis.bolotin@...ium.com>
Date: Tue, 31 Jul 2018 10:26:06 +0300

> +int qed_dcbx_get_priority_tc(struct qed_hwfn *p_hwfn, u8 pri, u8 *p_tc)
> +{
> +	struct qed_dcbx_get *dcbx_info;
> +	int rc;
> +
> +	if (pri >= QED_MAX_PFC_PRIORITIES) {
> +		DP_ERR(p_hwfn, "Invalid priority %d\n", pri);
> +		return -EINVAL;
> +	}
> +
> +	dcbx_info = kzalloc(sizeof(*dcbx_info), GFP_KERNEL);
> +	if (!dcbx_info)
> +		return -ENOMEM;
> +
> +	rc = qed_dcbx_query_params(p_hwfn, dcbx_info,
> +				   QED_DCBX_OPERATIONAL_MIB);

All the layering to probe these values is kinda crazy.

You go through all of the layers of the qed DCB code, capture
all of the DCBX operational state (it's a lot, it's not
trivial, and there is a ton of code that runs to place it
into this qed_dcbx_get structure).

Then you pick out _1_ value and throw the rest of it away.

This is kind of rediculious.  After the values have been
fetched, they sit in known qed driver data structures in
known locations.

There is therefore no reason to allocate memory for this
large structure.

Just provide an API in the DCBX code that 1) Makes sure the
info fetch has occurred and the software state is up to date
and 2) returns just the value you are looking for.

All of this indirection and extra work makes no sense at all.