[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTi=AkO0hUZ7o3uwX5zrxeiLi1TsLzu5QL=pUrAju@mail.gmail.com>
Date: Tue, 24 Aug 2010 21:53:49 +0800
From: Changli Gao <xiaosuo@...il.com>
To: Eric Dumazet <eric.dumazet@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: use skb_get_rxhash()
On Tue, Aug 24, 2010 at 6:21 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> 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)
>
Thanks. Got it. skb->rxhash should be only used when skb->sk is NULL,
and skb->rxhash should save the value copied from skb->sk->sk_hash at
that time.
if (skb->sk && skb->sk->sk_hash)
hash = skb->sk->sk_hash;
However, do we really need to check if skb->sk->sk_hash is zero? It is
a rare case, and this check may mislead someone to think a non zero
sk_hash means that sk_hash is set.
--
Regards,
Changli Gao(xiaosuo@...il.com)
--
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