netdev - Re: [PATCH] net: use skb_get

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]

Date:	Tue, 24 Aug 2010 12:21:02 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: use skb_get_rxhash()

Le mardi 24 août 2010 à 17:52 +0800, Changli Gao a écrit :
> Although it is rare the line contains skb->rxhash is executed:
> 
>  * The forwarded skbs should have the the rx queue recoreded.
>  * The local generated skbs should have non zero sk_hash in most of cases.
> 
> when it is executed, a non zero rxhash will be better.
> 
> Signed-off-by: Changli Gao <xiaosuo@...il.com>
> ---
>  net/core/dev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 859e30f..ea041fd 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2034,7 +2034,7 @@ u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
>  	if (skb->sk && skb->sk->sk_hash)
>  		hash = skb->sk->sk_hash;
>  	else
> -		hash = (__force u16) skb->protocol ^ skb->rxhash;
> +		hash = (__force u16) skb->protocol ^ skb_get_rxhash(skb);
>  	hash = jhash_1word(hash, hashrnd);
>  
>  	return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
> --

Instead of adding hacks and growing kernel code (considering your
previous patch), could we sit down and fix existing code, if really
needed ?

Spirit of existing code is that tcp/udp sockets have a sk_hash field,
and this feeds skb->rxhash in transmit path.

(check commit 87fd308c for an example of a fix)

I do think its better to chose a hash value per socket, instead of doing
it per packet (duplicated work, and slowdown mono threaded apps because
their working set will be shared by several queues, then several cpus)




--
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