netdev - RE: [PATCH net-next 1/4] enetc: Introduce basic PF and VF ENETC ethernet drivers

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: <VI1PR04MB48809E089371DA7DBA70AB1396D80@VI1PR04MB4880.eurprd04.prod.outlook.com>
Date:   Mon, 19 Nov 2018 15:25:58 +0000
From:   Claudiu Manoil <claudiu.manoil@....com>
To:     David Miller <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Catalin Horghidan <catalin.horghidan@....com>
Subject: RE: [PATCH net-next 1/4] enetc: Introduce basic PF and VF ENETC
 ethernet drivers

>-----Original Message-----
>From: David Miller <davem@...emloft.net>
>Sent: Saturday, November 17, 2018 10:08 PM
>To: Claudiu Manoil <claudiu.manoil@....com>
>Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Alexandru
>Marginean <alexandru.marginean@....com>; Catalin Horghidan
><catalin.horghidan@....com>
>Subject: Re: [PATCH net-next 1/4] enetc: Introduce basic PF and VF ENETC
>ethernet drivers
>
>From: Claudiu Manoil <claudiu.manoil@....com>
>Date: Thu, 15 Nov 2018 18:13:53 +0200
>
>> +	if (!nr_frags && !(flags & ENETC_TXBD_FLAGS_EX))
>> +		flags |= ENETC_TXBD_FLAGS_F;
>> +
>> +	txbd->flags = flags;
>> +
>> +	if (flags & ENETC_TXBD_FLAGS_EX) {
> ...
>> +		if (!nr_frags)
> ...
>> +	}
>> +
>> +	frag = &skb_shinfo(skb)->frags[0];
>> +	for (f = 0; f < nr_frags; f++, frag++) {
> ...
>> +	}
>> +
>> +	if (nr_frags)
>> +		/* last BD needs 'F' bit set */
>> +		txbd->flags = ENETC_TXBD_FLAGS_F;
>
>Isn't it so much simpler to just have an unconditional:
>
>	txbd->flags |= ENETC_TXBD_FLAGS_F;
>
>at the end of this code segment?
>
>Then all of this code conditional on nr_frags can go away.

True, but this code also avoids reading from the buffer descriptor memory
area (dma coherent), which is not cheap.  As it is now, the driver only writes
the txbd  (excepting the extension descriptor).  I'll look into how big this penalty
really is.  I'm also in favor of more readable code.

Thanks,
Claudiu