[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150425191702.GA2004@nanopsycho.orion>
Date: Sat, 25 Apr 2015 21:17:02 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Tom Herbert <tom@...bertland.com>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>,
Jamal Hadi Salim <jhs@...atatu.com>,
Thomas Graf <tgraf@...g.ch>, jesse@...ira.com, kaber@...sh.net,
Tom Herbert <therbert@...gle.com>, edumazet@...gle.com,
alexander.h.duyck@...hat.com,
Hannes Frederic Sowa <hannes@...essinduktion.org>,
ast@...mgrid.com, daniel@...earbox.net,
herbert@...dor.apana.org.au, cwang@...pensource.com,
john.fastabend@...il.com
Subject: Re: [patch net-next v4 RFC 12/15] flow_dissector: introduce support
for ipv6 addressses
Fri, Apr 24, 2015 at 07:28:05PM CEST, tom@...bertland.com wrote:
>Hi Jiri,
>
>Thanks for this work, I think it's a good direction! Some comments below...
>
>On Fri, Apr 24, 2015 at 8:51 AM, Jiri Pirko <jiri@...nulli.us> wrote:
...
>> enum flow_dissector_key_id {
>> FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
>> FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_addrs */
>> FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
>> FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
>> + FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs */
>>
>And we'll want to add VLAN ID, (GRE) key-id, IPv6 flow label, maybe a
>couple more.
Definitely. I plan to extend keys for those you mentioned and more in
future. Easy extendability is the goal of this patchset.
>
>> FLOW_DISSECTOR_KEY_MAX,
>> };
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 564288e..95e9a21 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -175,16 +175,29 @@ ipv6:
>> ip_proto = iph->nexthdr;
>> nhoff += sizeof(struct ipv6hdr);
>>
>> - if (!skb_flow_dissector_uses_key(flow_dissector,
>> - FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS))
>> - break;
>> - key_addrs = skb_flow_dissector_target(flow_dissector,
>> - FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
>> - target_container);
>> + if (skb_flow_dissector_uses_key(flow_dissector,
>> + FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
>> + key_addrs = skb_flow_dissector_target(flow_dissector,
>> + FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
>> + target_container);
>>
>> - key_addrs->src = (__force __be32)ipv6_addr_hash(&iph->saddr);
>> - key_addrs->dst = (__force __be32)ipv6_addr_hash(&iph->daddr);
>> + key_addrs->src = (__force __be32)ipv6_addr_hash(&iph->saddr);
>> + key_addrs->dst = (__force __be32)ipv6_addr_hash(&iph->daddr);
>> + goto flow_label;
>> + }
>
>So this is still folding the IPv6 addresses so that that we can fit
>into jhash_3words? Can we address this now and include the full IPv6
>address in the hash? I would propose that we extend the flow_keys
>structure (which I think you may already be doing) for full IPv6
>address, VLAN, flow label, etc., and then produce a hash across that
>whole structure. jhash2 can be used on a structure. jhash is actually
>a very efficient hash performance wise-- the rest of flow dissection
>is likely the dominant cost anyway. We should also minimize calls to
>flow_dissector, I would propose it should be called at most once per
>packet-- I'll be reposting patches to fix that in the various qdiscs.
This hash computation is currently present in kernel. I just ported that
to new programable dissector framework.
I'm not sure how possible is the flow_keys structure extension. In this
patchset, I just maintain the current size and usage. But the thing is
that every individual caller of flow_dissect can use their own struct
which includes exactly what he needs.
Regarding the flow_dissect call reduction, I agree that it is something
we need. The question is how to pass the dissect results through the
code. For example, sch_choke uses qdisc_cb_private for passing flow_keys
struct. However qdisc_skb_cb can by only 20 bytes long (QDISC_CB_PRIV_LEN)
This can be however resolver independently of this patchset.
--
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