netdev - Re: [net-next PATCH v3] ibmvnic: map L2/L3/L4 header descriptors to firmware

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]

Message-Id: <20160225.001234.830392805325286378.davem@davemloft.net>
Date:	Thu, 25 Feb 2016 00:12:34 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	tlfalcon@...ux.vnet.ibm.com
Cc:	netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [net-next PATCH v3] ibmvnic: map L2/L3/L4 header descriptors
 to firmware

From: Thomas Falcon <tlfalcon@...ux.vnet.ibm.com>
Date: Wed, 24 Feb 2016 21:34:43 -0600

> +
> +static union sub_crq *build_hdr_descs_arr(struct sk_buff *skb,
> +					  int *num_entries,
> +					  union sub_crq subcrq, u8 hdr_field)
> +{
> +	unsigned char *hdr_data;
> +	union sub_crq *entries;
> +	int hdr_len[3] = {0};
> +	int tot_len;
> +
> +	hdr_data = build_hdr_data(hdr_field, skb, hdr_len, &tot_len);
> +	if (!hdr_data)
> +		return NULL;
> +
> +	*num_entries += calc_num_hdr_descs(tot_len);
> +
> +	entries = alloc_scrq_array(*num_entries, subcrq);

Doing an extra memory allocation for every transmit packet is going
to kill performance.  We are spending enormous amounts of effort
trying to eliminate the bare minimum memory allocations that occur
as-is because that cost is what dominates and this patch is going
to make it even worse for this device.

I don't care what offloads this enables, the implementation is
unacceptable from a performance standpoint.

Also, you should only add this new code when the offloads that make
use of the feature are added as well, otherwise it is unused bloat.

I am not applying this patch, there are so many things wrong with it.

Sorry.