[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1282645262.2477.64.camel@edumazet-laptop>
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
Powered by blists - more mailing lists