netdev - Re: [PATCH v5] rps: Receive Packet Steering

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B5008F5.6090007@gmail.com>
Date:	Fri, 15 Jan 2010 07:19:33 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
CC:	Tom Herbert <therbert@...gle.com>, davem@...emloft.net,
	netdev@...r.kernel.org,
	Netfilter Developer Mailing List 
	<netfilter-devel@...r.kernel.org>
Subject: Re: [PATCH v5] rps: Receive Packet Steering

Le 15/01/2010 03:22, Changli Gao a écrit :
> On Fri, Jan 15, 2010 at 5:56 AM, Tom Herbert <therbert@...gle.com> wrote:
>> +
>> +       if (skb->rxhash)
>> +               goto got_hash; /* Skip hash computation on packet header */
>> +
>> +       switch (skb->protocol) {
>> +       case __constant_htons(ETH_P_IP):
>> +               if (!pskb_may_pull(skb, sizeof(*ip)))
>> +                       goto done;
>> +
>> +               ip = (struct iphdr *) skb->data;
>> +               ip_proto = ip->protocol;
>> +               addr1 = ip->saddr;
>> +               addr2 = ip->daddr;
>> +               ihl = ip->ihl;
>> +               break;
>> +       case __constant_htons(ETH_P_IPV6):
>> +               if (!pskb_may_pull(skb, sizeof(*ip6)))
>> +                       goto done;
>> +
>> +               ip6 = (struct ipv6hdr *) skb->data;
>> +               ip_proto = ip6->nexthdr;
> This code can't work, when there are extra headers. ipv6_skip_exthdr()
> can be used to get the l4 header.

Could you give exact code please ?

> 
>> +               addr1 = ip6->saddr.s6_addr32[3];
>> +               addr2 = ip6->daddr.s6_addr32[3];
>> +               ihl = (40 >> 2);
>> +               break;
>> +       default:
>> +               goto done;
>> +       }
>> +       ports = 0;
>> +       switch (ip_proto) {
>> +       case IPPROTO_TCP:
>> +       case IPPROTO_UDP:
>> +       case IPPROTO_DCCP:
>> +       case IPPROTO_ESP:
>> +       case IPPROTO_AH:
>> +       case IPPROTO_SCTP:
>> +       case IPPROTO_UDPLITE:
>> +               if (pskb_may_pull(skb, (ihl * 4) + 4))
>> +                       ports = *((u32 *) (skb->data + (ihl * 4)));
>> +               break;
>> +
>> +       default:
>> +               break;
>> +       }
>> +
>> +       skb->rxhash = jhash_3words(addr1, addr2, ports, hashrnd);
> For connection based packet processing, such as netfilter,
> distributing the packets in two directions into one CPU will reduce
> cache miss, when NAT isn't used. I think the code bellow will help:
> if (addr1 > addr2)
>   swap(addr1, addr2);

Yes, I already gave this hint in a previous review, but this adds a test
and I suspect Google is not using NAT :)

> 
>> +       if (!skb->rxhash)
>> +               skb->rxhash = 1;
> 
> Why not put the above code into a new function, and add more protocols
> support, such as 802.1Q.  Though rxhash is based on 4-tuple, I think
> netfilter will benefit from it.
> 

Sure, this can be done in a followup patch.

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