netdev - Re: [PATCH net-next] sfc: add support for skb->xmit

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:	Wed, 22 Oct 2014 13:36:21 +0100
From:	Edward Cree <ecree@...arflare.com>
To:	Ben Hutchings <ben@...adent.org.uk>
CC:	Robert Stonehouse <rstonehouse@...arflare.com>,
	Daniel Borkmann <dborkman@...hat.com>, <davem@...emloft.net>,
	<nikolay@...hat.com>, <netdev@...r.kernel.org>,
	Shradha Shah <sshah@...arflare.com>,
	Jon Cooper <jcooper@...arflare.com>,
	linux-net-drivers <linux-net-drivers@...arflare.com>
Subject: Re: [PATCH net-next] sfc: add support for skb->xmit_more

On 22/10/14 09:58, Ben Hutchings wrote:
> On Fri, 2014-10-17 at 15:32 +0100, Edward Cree wrote:
> [...]
>> @@ -351,7 +343,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
>>  	unsigned short dma_flags;
>>  	int i = 0;
>>  
>> -	EFX_BUG_ON_PARANOID(tx_queue->write_count != tx_queue->insert_count);
>> +	EFX_BUG_ON_PARANOID(tx_queue->write_count > tx_queue->insert_count);
> [...]
>
> Doesn't this break after 2^32 descriptors?  It seems like you would need
> a similar comparison to time_after(); possibly:
>
> 	EFX_BUG_ON_PARANOID((int)(tx_queue->write_count - tx_queue->insert_count) > 0);
>
> Ben.
>
Yes, Jon already spotted that.  We just diked it out - see "[PATCH net]
sfc: remove incorrect EFX_BUG_ON_PARANOID check"
http://patchwork.ozlabs.org/patch/401503/
But your suggestion is a good one; feel free to post as a new patch on
that thread.
-Edward
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html