David Miller wrote:
> It just xor hashes over IPv4/IPv6 addresses and ports of transport.
>
> The only assumption it makes is that skb_network_header() is set
> correctly.
>
Hey David,
Sorry for the late reply, I didn't have time to look at multiqueue tx
details before. I have some curiosity-questions about your hashing.
> +static u16 simple_tx_hash(struct net_device *dev, struct sk_buff *skb)
> +{
> + u32 *addr, *ports, hash, ihl;
> + u8 ip_proto;
> + int alen;
> +
> + switch (skb->protocol) {
> + case __constant_htons(ETH_P_IP):
> + ip_proto = ip_hdr(skb)->protocol;
> + addr = &ip_hdr(skb)->saddr;
> + ihl = ip_hdr(skb)->ihl;
> + alen = 2;
> + break;
> + case __constant_htons(ETH_P_IPV6):
> + ip_proto = ipv6_hdr(skb)->nexthdr;
> + addr = &ipv6_hdr(skb)->saddr.s6_addr32[0];
> + ihl = (40 >> 2);
> + alen = 8;
> + break;
> + default:
> + return 0;
> + }
>
What's your plan for other protocols here? Should we add an optional
tx_hash() method in struct packet_type?
> @@ -1672,6 +1722,8 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
>
> if (dev->select_queue)
> queue_index = dev->select_queue(dev, skb);
> + else if (dev->real_num_tx_queues > 1)
> + queue_index = simple_tx_hash(dev, skb);
>
Some devices might will add some protocol specific hashing (IP and IPv6)
that could look like what simple_tx_hash() does above, right? But if we
add some new protocols in sample_tx_hash(), it will be ignored by these
devices, and we might have to update the hashing in all select_queue()
methods as well.
So I wonder if we shouldn't change all this into:
1) simple_tx_hash(skb) returns a big protocol-independent integer (like
a concatenation or basic xor of IP proto/addr/...). It basically just
converts the skb header in something flat and proto-independent.
2.a) if the device provides select_queue(), we pass this integer to it.
select_queue() either uses the skb headers if it really wants to hash IP
or IPv6 precisely, or uses our protocol-independent integer and just
hash it in a dumb way without caring about the protocol hidden behind it.
2.b) if there's no select_queue(), we hash the big integer depending on
the number of tx queues
This way, we can easily add some protocol specific hashing to all
drivers without having to modify select_queue() everywhere. Of course,
if we don't care about hashing anything but IP/IPv6, it doesn't matter :)
Brice
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
