linux-kernel - Re: [net-next v5 1/6] net: marvell: prestera: Add driver for Prestera family ASIC devices

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: <20200825.172003.1417643181819895272.davem@davemloft.net>
Date:   Tue, 25 Aug 2020 17:20:03 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     vadym.kochan@...ision.eu
Cc:     kuba@...nel.org, jiri@...lanox.com, idosch@...lanox.com,
        andrew@...n.ch, oleksandr.mazur@...ision.eu,
        serhiy.boiko@...ision.eu, serhiy.pshyk@...ision.eu,
        volodymyr.mytnyk@...ision.eu, taras.chornyi@...ision.eu,
        andrii.savka@...ision.eu, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, andy.shevchenko@...il.com,
        mickeyr@...vell.com
Subject: Re: [net-next v5 1/6] net: marvell: prestera: Add driver for
 Prestera family ASIC devices

From: Vadym Kochan <vadym.kochan@...ision.eu>
Date: Tue, 25 Aug 2020 15:20:08 +0300

> +int prestera_dsa_parse(struct prestera_dsa *dsa, const u8 *dsa_buf)
> +{
> +	__be32 *dsa_words = (__be32 *)dsa_buf;
> +	enum prestera_dsa_cmd cmd;
> +	u32 words[4] = { 0 };
> +	u32 field;
> +
> +	words[0] = ntohl(dsa_words[0]);
> +	words[1] = ntohl(dsa_words[1]);
> +	words[2] = ntohl(dsa_words[2]);
> +	words[3] = ntohl(dsa_words[3]);

All 4 elements of words[] are explicitly and unconditionally set to something,
so you don't need the "= { 0 }" initializer.

> +	INIT_LIST_HEAD(&sw->port_list);

What locking protects this list?

> +	new_skb = alloc_skb(len, GFP_ATOMIC | GFP_DMA);
> +	if (!new_skb)
> +		goto err_alloc_skb;

This seems very costly to do copies on every packet.  There should be
something in the dma_*() API that would allow you to avoid this.

And since you just need the buffer to DMA map it into the device,
allocating an entire SKB object is overkill.  You can instead just
allocate a ring of TX bounce buffers once, and then you just copy
each packet there.  No allocations per packet.