* [PATCH] net: Initialize entire flowi struct
@ 2011-08-31 16:05 David Ward
2011-08-31 20:51 ` Julian Anastasov
0 siblings, 1 reply; 14+ messages in thread
From: David Ward @ 2011-08-31 16:05 UTC (permalink / raw)
To: netdev; +Cc: David Ward
The entire flowi struct needs to be initialized by afinfo->decode_session,
because flow_hash_code operates over the entire struct and may otherwise
return different hash values for what is intended to be the same key.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
net/ipv4/xfrm4_policy.c | 2 +-
net/ipv6/xfrm6_policy.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index fc5368a..afce24d 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -114,7 +114,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
struct flowi4 *fl4 = &fl->u.ip4;
- memset(fl4, 0, sizeof(struct flowi4));
+ memset(fl, 0, sizeof(struct flowi));
fl4->flowi4_mark = skb->mark;
if (!ip_is_fragment(iph)) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index d879f7e..9088d38 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -129,7 +129,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
- memset(fl6, 0, sizeof(struct flowi6));
+ memset(fl, 0, sizeof(struct flowi));
fl6->flowi6_mark = skb->mark;
ipv6_addr_copy(&fl6->daddr, reverse ? &hdr->saddr : &hdr->daddr);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Initialize entire flowi struct
2011-08-31 20:51 ` Julian Anastasov
@ 2011-08-31 20:47 ` David Miller
2011-09-01 13:34 ` Ward, David - 0663 - MITLL
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2011-08-31 20:47 UTC (permalink / raw)
To: ja; +Cc: david.ward, netdev
From: Julian Anastasov <ja@ssi.bg>
Date: Wed, 31 Aug 2011 23:51:32 +0300 (EEST)
> Such change will cause problems for callers that
> use flowi4 in stack. Examples:
>
> ip_route_me_harder
> icmp_route_lookup
Right.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Initialize entire flowi struct
2011-08-31 16:05 [PATCH] net: Initialize entire flowi struct David Ward
@ 2011-08-31 20:51 ` Julian Anastasov
2011-08-31 20:47 ` David Miller
2011-09-01 13:34 ` Ward, David - 0663 - MITLL
0 siblings, 2 replies; 14+ messages in thread
From: Julian Anastasov @ 2011-08-31 20:51 UTC (permalink / raw)
To: David Ward; +Cc: netdev
Hello,
On Wed, 31 Aug 2011, David Ward wrote:
> The entire flowi struct needs to be initialized by afinfo->decode_session,
> because flow_hash_code operates over the entire struct and may otherwise
> return different hash values for what is intended to be the same key.
Such change will cause problems for callers that
use flowi4 in stack. Examples:
ip_route_me_harder
icmp_route_lookup
Not sure if adding size as parameter to flow_hash_code
is better approach. May be flow_cache_lookup needs to
determine size from family that can be used for flow_hash_code,
flow_key_compare and the memcpy(&fle->key, key, sizeof(*key))
after fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC). The
question is how to get size by family.
> Signed-off-by: David Ward <david.ward@ll.mit.edu>
> ---
> net/ipv4/xfrm4_policy.c | 2 +-
> net/ipv6/xfrm6_policy.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
> index fc5368a..afce24d 100644
> --- a/net/ipv4/xfrm4_policy.c
> +++ b/net/ipv4/xfrm4_policy.c
> @@ -114,7 +114,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
> u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
> struct flowi4 *fl4 = &fl->u.ip4;
>
> - memset(fl4, 0, sizeof(struct flowi4));
> + memset(fl, 0, sizeof(struct flowi));
> fl4->flowi4_mark = skb->mark;
>
> if (!ip_is_fragment(iph)) {
> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index d879f7e..9088d38 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -129,7 +129,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
> const unsigned char *nh = skb_network_header(skb);
> u8 nexthdr = nh[IP6CB(skb)->nhoff];
>
> - memset(fl6, 0, sizeof(struct flowi6));
> + memset(fl, 0, sizeof(struct flowi));
> fl6->flowi6_mark = skb->mark;
>
> ipv6_addr_copy(&fl6->daddr, reverse ? &hdr->saddr : &hdr->daddr);
> --
> 1.7.4.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Initialize entire flowi struct
2011-08-31 20:51 ` Julian Anastasov
2011-08-31 20:47 ` David Miller
@ 2011-09-01 13:34 ` Ward, David - 0663 - MITLL
2011-09-03 7:27 ` Julian Anastasov
1 sibling, 1 reply; 14+ messages in thread
From: Ward, David - 0663 - MITLL @ 2011-09-01 13:34 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]
Hi Julian,
On 08/31/2011 04:51 PM, Julian Anastasov wrote:
> On Wed, 31 Aug 2011, David Ward wrote
>> The entire flowi struct needs to be initialized by afinfo->decode_session,
>> because flow_hash_code operates over the entire struct and may otherwise
>> return different hash values for what is intended to be the same key.
> Such change will cause problems for callers that
> use flowi4 in stack. Examples:
>
> ip_route_me_harder
> icmp_route_lookup
Thanks for pointing this out.
> Not sure if adding size as parameter to flow_hash_code
> is better approach. May be flow_cache_lookup needs to
> determine size from family that can be used for flow_hash_code,
> flow_key_compare and the memcpy(&fle->key, key, sizeof(*key))
> after fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC).
Makes sense to me. However should we just replace flow_key_compare with
memcmp then, since the assumptions about constant size and alignment
will no longer apply? Or should there be a separate flow_key_compare
function for each family, and have all of the flowi* structures become
__attribute__((__aligned__(BITS_PER_LONG/8))) ?
David
> The question is how to get size by family.
>
>> Signed-off-by: David Ward<david.ward@ll.mit.edu>
>> ---
>> net/ipv4/xfrm4_policy.c | 2 +-
>> net/ipv6/xfrm6_policy.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
>> index fc5368a..afce24d 100644
>> --- a/net/ipv4/xfrm4_policy.c
>> +++ b/net/ipv4/xfrm4_policy.c
>> @@ -114,7 +114,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
>> u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
>> struct flowi4 *fl4 =&fl->u.ip4;
>>
>> - memset(fl4, 0, sizeof(struct flowi4));
>> + memset(fl, 0, sizeof(struct flowi));
>> fl4->flowi4_mark = skb->mark;
>>
>> if (!ip_is_fragment(iph)) {
>> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
>> index d879f7e..9088d38 100644
>> --- a/net/ipv6/xfrm6_policy.c
>> +++ b/net/ipv6/xfrm6_policy.c
>> @@ -129,7 +129,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
>> const unsigned char *nh = skb_network_header(skb);
>> u8 nexthdr = nh[IP6CB(skb)->nhoff];
>>
>> - memset(fl6, 0, sizeof(struct flowi6));
>> + memset(fl, 0, sizeof(struct flowi));
>> fl6->flowi6_mark = skb->mark;
>>
>> ipv6_addr_copy(&fl6->daddr, reverse ?&hdr->saddr :&hdr->daddr);
>> --
>> 1.7.4.1
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4184 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Initialize entire flowi struct
2011-09-01 13:34 ` Ward, David - 0663 - MITLL
@ 2011-09-03 7:27 ` Julian Anastasov
2011-09-04 13:07 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Julian Anastasov @ 2011-09-03 7:27 UTC (permalink / raw)
To: Ward, David - 0663 - MITLL; +Cc: David Miller, netdev
Hello,
On Thu, 1 Sep 2011, Ward, David - 0663 - MITLL wrote:
> > Not sure if adding size as parameter to flow_hash_code
> > is better approach. May be flow_cache_lookup needs to
> > determine size from family that can be used for flow_hash_code,
> > flow_key_compare and the memcpy(&fle->key, key, sizeof(*key))
> > after fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC).
>
> Makes sense to me. However should we just replace flow_key_compare with
> memcmp then, since the assumptions about constant size and alignment will no
> longer apply? Or should there be a separate flow_key_compare function for
> each family, and have all of the flowi* structures become
> __attribute__((__aligned__(BITS_PER_LONG/8))) ?
I don't know this code well but I guess memcmp is
not preferred. IMHO, as the callers provide per-family
structures and we do not want to change that, these
structures must be aligned to long type as required by
flow_compare_t and jhash2 (at least u32) usage. The second
option is to use memcmp and jhash instead of jhash2 to
avoid such alignment but I guess other developers will
oppose it. More opinions are needed here.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs
2011-09-03 7:27 ` Julian Anastasov
@ 2011-09-04 13:07 ` David Ward
2011-09-04 13:07 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
2011-09-04 13:07 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2 siblings, 0 replies; 14+ messages in thread
From: David Ward @ 2011-09-04 13:07 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, David Ward
These fixes to the flow cache are needed with the conversion to AF-specific
flowi structs. They are written so as to avoid introducing AF-specific code
into net/core/flow.c.
Note that __xfrm_policy_check (in net/xfrm/xfrm_policy.c) still allocates a
struct flowi on the stack and passes it to flow_cache_lookup. My understanding
is that since this is on the stack, this will not be aligned, and therefore it
will cause problems with flow_hash_code and flow_key_compare. Is that correct?
Signed-off-by: David Ward <david.ward@ll.mit.edu>
David Ward (2):
net: Align AF-specific flowi structs to long
net: Handle different key sizes between address families in flow
cache
include/net/flow.h | 25 ++++++++++++++++++++++---
net/core/flow.c | 28 ++++++++++++++++------------
2 files changed, 38 insertions(+), 15 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] net: Align AF-specific flowi structs to long
2011-09-03 7:27 ` Julian Anastasov
2011-09-04 13:07 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
@ 2011-09-04 13:07 ` David Ward
2011-09-16 22:57 ` David Miller
2011-09-04 13:07 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2 siblings, 1 reply; 14+ messages in thread
From: David Ward @ 2011-09-04 13:07 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, David Ward
AF-specific flowi structs are now passed to flow_key_compare, which must
also be aligned to a long.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/flow.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 78113da..2ec377d 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -68,7 +68,7 @@ struct flowi4 {
#define fl4_ipsec_spi uli.spi
#define fl4_mh_type uli.mht.type
#define fl4_gre_key uli.gre_key
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
__u32 mark, __u8 tos, __u8 scope,
@@ -112,7 +112,7 @@ struct flowi6 {
#define fl6_ipsec_spi uli.spi
#define fl6_mh_type uli.mht.type
#define fl6_gre_key uli.gre_key
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
struct flowidn {
struct flowi_common __fl_common;
@@ -127,7 +127,7 @@ struct flowidn {
union flowi_uli uli;
#define fld_sport uli.ports.sport
#define fld_dport uli.ports.dport
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
struct flowi {
union {
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] net: Handle different key sizes between address families in flow cache
2011-09-03 7:27 ` Julian Anastasov
2011-09-04 13:07 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
2011-09-04 13:07 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
@ 2011-09-04 13:07 ` David Ward
2011-09-04 15:34 ` Michał Mirosław
2011-09-16 22:57 ` David Miller
2 siblings, 2 replies; 14+ messages in thread
From: David Ward @ 2011-09-04 13:07 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, David Ward
With the conversion of struct flowi to a union of AF-specific structs, some
operations on the flow cache need to account for the exact size of the key.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/flow.h | 19 +++++++++++++++++++
net/core/flow.c | 28 ++++++++++++++++------------
2 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 2ec377d..99119f6 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -7,6 +7,7 @@
#ifndef _NET_FLOW_H
#define _NET_FLOW_H
+#include <linux/socket.h>
#include <linux/in6.h>
#include <linux/atomic.h>
@@ -161,6 +162,24 @@ static inline struct flowi *flowidn_to_flowi(struct flowidn *fldn)
return container_of(fldn, struct flowi, u.dn);
}
+typedef unsigned long flow_compare_t;
+
+static inline size_t flowi_size(u16 family)
+{
+ switch (family) {
+ case AF_INET:
+ BUILD_BUG_ON(sizeof(struct flowi4) % sizeof(flow_compare_t));
+ return sizeof(struct flowi4);
+ case AF_INET6:
+ BUILD_BUG_ON(sizeof(struct flowi6) % sizeof(flow_compare_t));
+ return sizeof(struct flowi6);
+ case AF_DECnet:
+ BUILD_BUG_ON(sizeof(struct flowidn) % sizeof(flow_compare_t));
+ return sizeof(struct flowidn);
+ }
+ return 0;
+}
+
#define FLOW_DIR_IN 0
#define FLOW_DIR_OUT 1
#define FLOW_DIR_FWD 2
diff --git a/net/core/flow.c b/net/core/flow.c
index bf32c33..1545445 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -172,26 +172,24 @@ static void flow_new_hash_rnd(struct flow_cache *fc,
static u32 flow_hash_code(struct flow_cache *fc,
struct flow_cache_percpu *fcp,
- const struct flowi *key)
+ const struct flowi *key,
+ size_t keysize)
{
const u32 *k = (const u32 *) key;
- return jhash2(k, (sizeof(*key) / sizeof(u32)), fcp->hash_rnd)
+ return jhash2(k, (keysize / sizeof(u32)), fcp->hash_rnd)
& (flow_cache_hash_size(fc) - 1);
}
-typedef unsigned long flow_compare_t;
-
/* I hear what you're saying, use memcmp. But memcmp cannot make
* important assumptions that we can here, such as alignment and
- * constant size.
+ * size.
*/
-static int flow_key_compare(const struct flowi *key1, const struct flowi *key2)
+static int flow_key_compare(const struct flowi *key1, const struct flowi *key2,
+ size_t keysize)
{
const flow_compare_t *k1, *k1_lim, *k2;
- const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
-
- BUILD_BUG_ON(sizeof(struct flowi) % sizeof(flow_compare_t));
+ const int n_elem = keysize / sizeof(flow_compare_t);
k1 = (const flow_compare_t *) key1;
k1_lim = k1 + n_elem;
@@ -215,6 +213,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
struct flow_cache_entry *fle, *tfle;
struct hlist_node *entry;
struct flow_cache_object *flo;
+ size_t keysize;
unsigned int hash;
local_bh_disable();
@@ -222,6 +221,11 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
fle = NULL;
flo = NULL;
+
+ keysize = flowi_size(family);
+ if (!keysize)
+ goto nocache;
+
/* Packet really early in init? Making flow_cache_init a
* pre-smp initcall would solve this. --RR */
if (!fcp->hash_table)
@@ -230,11 +234,11 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
if (fcp->hash_rnd_recalc)
flow_new_hash_rnd(fc, fcp);
- hash = flow_hash_code(fc, fcp, key);
+ hash = flow_hash_code(fc, fcp, key, keysize);
hlist_for_each_entry(tfle, entry, &fcp->hash_table[hash], u.hlist) {
if (tfle->family == family &&
tfle->dir == dir &&
- flow_key_compare(key, &tfle->key) == 0) {
+ flow_key_compare(key, &tfle->key, keysize) == 0) {
fle = tfle;
break;
}
@@ -248,7 +252,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
if (fle) {
fle->family = family;
fle->dir = dir;
- memcpy(&fle->key, key, sizeof(*key));
+ memcpy(&fle->key, key, keysize);
fle->object = NULL;
hlist_add_head(&fle->u.hlist, &fcp->hash_table[hash]);
fcp->hash_count++;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] net: Handle different key sizes between address families in flow cache
2011-09-04 13:07 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
@ 2011-09-04 15:34 ` Michał Mirosław
2011-09-06 2:47 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
` (2 more replies)
2011-09-16 22:57 ` David Miller
1 sibling, 3 replies; 14+ messages in thread
From: Michał Mirosław @ 2011-09-04 15:34 UTC (permalink / raw)
To: David Ward; +Cc: netdev, Julian Anastasov
2011/9/4 David Ward <david.ward@ll.mit.edu>:
> With the conversion of struct flowi to a union of AF-specific structs, some
> operations on the flow cache need to account for the exact size of the key.
[...]
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
[...]
> +typedef unsigned long flow_compare_t;
> +
> +static inline size_t flowi_size(u16 family)
> +{
> + switch (family) {
> + case AF_INET:
> + BUILD_BUG_ON(sizeof(struct flowi4) % sizeof(flow_compare_t));
> + return sizeof(struct flowi4);
> + case AF_INET6:
> + BUILD_BUG_ON(sizeof(struct flowi6) % sizeof(flow_compare_t));
> + return sizeof(struct flowi6);
> + case AF_DECnet:
> + BUILD_BUG_ON(sizeof(struct flowidn) % sizeof(flow_compare_t));
> + return sizeof(struct flowidn);
> + }
> + return 0;
> +}
Since most called user (flow_key_compare) uses returned value didided
by sizeof(flow_compare_t), you could just return divided value here
and save some shift operations by it.
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs
2011-09-04 15:34 ` Michał Mirosław
@ 2011-09-06 2:47 ` David Ward
2011-09-06 2:47 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
2011-09-06 2:47 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2 siblings, 0 replies; 14+ messages in thread
From: David Ward @ 2011-09-06 2:47 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, Michał Mirosław, David Ward
v2: Return the length of the flow key as a multiple of sizeof(flow_compare_t),
to reduce the number of shift operations.
These fixes to the flow cache are needed with the conversion to AF-specific
flowi structs. They are written so as to avoid introducing AF-specific code
into net/core/flow.c.
Note that __xfrm_policy_check (in net/xfrm/xfrm_policy.c) still allocates a
struct flowi on the stack and passes it to flow_cache_lookup. My understanding
is that since this is on the stack, this will not be aligned, and therefore it
will cause problems with flow_hash_code and flow_key_compare. Is that correct?
Signed-off-by: David Ward <david.ward@ll.mit.edu>
David Ward (2):
net: Align AF-specific flowi structs to long
net: Handle different key sizes between address families in flow
cache
include/net/flow.h | 25 ++++++++++++++++++++++---
net/core/flow.c | 31 +++++++++++++++++--------------
2 files changed, 39 insertions(+), 17 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] net: Align AF-specific flowi structs to long
2011-09-04 15:34 ` Michał Mirosław
2011-09-06 2:47 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
@ 2011-09-06 2:47 ` David Ward
2011-09-06 2:47 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2 siblings, 0 replies; 14+ messages in thread
From: David Ward @ 2011-09-06 2:47 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, Michał Mirosław, David Ward
AF-specific flowi structs are now passed to flow_key_compare, which must
also be aligned to a long.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/flow.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 78113da..2ec377d 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -68,7 +68,7 @@ struct flowi4 {
#define fl4_ipsec_spi uli.spi
#define fl4_mh_type uli.mht.type
#define fl4_gre_key uli.gre_key
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
__u32 mark, __u8 tos, __u8 scope,
@@ -112,7 +112,7 @@ struct flowi6 {
#define fl6_ipsec_spi uli.spi
#define fl6_mh_type uli.mht.type
#define fl6_gre_key uli.gre_key
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
struct flowidn {
struct flowi_common __fl_common;
@@ -127,7 +127,7 @@ struct flowidn {
union flowi_uli uli;
#define fld_sport uli.ports.sport
#define fld_dport uli.ports.dport
-};
+} __attribute__((__aligned__(BITS_PER_LONG/8)));
struct flowi {
union {
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] net: Handle different key sizes between address families in flow cache
2011-09-04 15:34 ` Michał Mirosław
2011-09-06 2:47 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
2011-09-06 2:47 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
@ 2011-09-06 2:47 ` David Ward
2 siblings, 0 replies; 14+ messages in thread
From: David Ward @ 2011-09-06 2:47 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, Michał Mirosław, David Ward
With the conversion of struct flowi to a union of AF-specific structs, some
operations on the flow cache need to account for the exact size of the key.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/flow.h | 19 +++++++++++++++++++
net/core/flow.c | 31 +++++++++++++++++--------------
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 2ec377d..a094477 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -7,6 +7,7 @@
#ifndef _NET_FLOW_H
#define _NET_FLOW_H
+#include <linux/socket.h>
#include <linux/in6.h>
#include <linux/atomic.h>
@@ -161,6 +162,24 @@ static inline struct flowi *flowidn_to_flowi(struct flowidn *fldn)
return container_of(fldn, struct flowi, u.dn);
}
+typedef unsigned long flow_compare_t;
+
+static inline size_t flow_key_size(u16 family)
+{
+ switch (family) {
+ case AF_INET:
+ BUILD_BUG_ON(sizeof(struct flowi4) % sizeof(flow_compare_t));
+ return sizeof(struct flowi4) / sizeof(flow_compare_t);
+ case AF_INET6:
+ BUILD_BUG_ON(sizeof(struct flowi6) % sizeof(flow_compare_t));
+ return sizeof(struct flowi6) / sizeof(flow_compare_t);
+ case AF_DECnet:
+ BUILD_BUG_ON(sizeof(struct flowidn) % sizeof(flow_compare_t));
+ return sizeof(struct flowidn) / sizeof(flow_compare_t);
+ }
+ return 0;
+}
+
#define FLOW_DIR_IN 0
#define FLOW_DIR_OUT 1
#define FLOW_DIR_FWD 2
diff --git a/net/core/flow.c b/net/core/flow.c
index bf32c33..2d93a37 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -172,29 +172,26 @@ static void flow_new_hash_rnd(struct flow_cache *fc,
static u32 flow_hash_code(struct flow_cache *fc,
struct flow_cache_percpu *fcp,
- const struct flowi *key)
+ const struct flowi *key,
+ size_t keysize)
{
const u32 *k = (const u32 *) key;
+ const u32 length = keysize * sizeof(flow_compare_t) / sizeof(u32);
- return jhash2(k, (sizeof(*key) / sizeof(u32)), fcp->hash_rnd)
+ return jhash2(k, length, fcp->hash_rnd)
& (flow_cache_hash_size(fc) - 1);
}
-typedef unsigned long flow_compare_t;
-
/* I hear what you're saying, use memcmp. But memcmp cannot make
- * important assumptions that we can here, such as alignment and
- * constant size.
+ * important assumptions that we can here, such as alignment.
*/
-static int flow_key_compare(const struct flowi *key1, const struct flowi *key2)
+static int flow_key_compare(const struct flowi *key1, const struct flowi *key2,
+ size_t keysize)
{
const flow_compare_t *k1, *k1_lim, *k2;
- const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
-
- BUILD_BUG_ON(sizeof(struct flowi) % sizeof(flow_compare_t));
k1 = (const flow_compare_t *) key1;
- k1_lim = k1 + n_elem;
+ k1_lim = k1 + keysize;
k2 = (const flow_compare_t *) key2;
@@ -215,6 +212,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
struct flow_cache_entry *fle, *tfle;
struct hlist_node *entry;
struct flow_cache_object *flo;
+ size_t keysize;
unsigned int hash;
local_bh_disable();
@@ -222,6 +220,11 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
fle = NULL;
flo = NULL;
+
+ keysize = flow_key_size(family);
+ if (!keysize)
+ goto nocache;
+
/* Packet really early in init? Making flow_cache_init a
* pre-smp initcall would solve this. --RR */
if (!fcp->hash_table)
@@ -230,11 +233,11 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
if (fcp->hash_rnd_recalc)
flow_new_hash_rnd(fc, fcp);
- hash = flow_hash_code(fc, fcp, key);
+ hash = flow_hash_code(fc, fcp, key, keysize);
hlist_for_each_entry(tfle, entry, &fcp->hash_table[hash], u.hlist) {
if (tfle->family == family &&
tfle->dir == dir &&
- flow_key_compare(key, &tfle->key) == 0) {
+ flow_key_compare(key, &tfle->key, keysize) == 0) {
fle = tfle;
break;
}
@@ -248,7 +251,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
if (fle) {
fle->family = family;
fle->dir = dir;
- memcpy(&fle->key, key, sizeof(*key));
+ memcpy(&fle->key, key, keysize * sizeof(flow_compare_t));
fle->object = NULL;
hlist_add_head(&fle->u.hlist, &fcp->hash_table[hash]);
fcp->hash_count++;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] net: Align AF-specific flowi structs to long
2011-09-04 13:07 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
@ 2011-09-16 22:57 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2011-09-16 22:57 UTC (permalink / raw)
To: david.ward; +Cc: netdev, ja
From: David Ward <david.ward@ll.mit.edu>
Date: Sun, 4 Sep 2011 09:07:20 -0400
> AF-specific flowi structs are now passed to flow_key_compare, which must
> also be aligned to a long.
>
> Signed-off-by: David Ward <david.ward@ll.mit.edu>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] net: Handle different key sizes between address families in flow cache
2011-09-04 13:07 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2011-09-04 15:34 ` Michał Mirosław
@ 2011-09-16 22:57 ` David Miller
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2011-09-16 22:57 UTC (permalink / raw)
To: david.ward; +Cc: netdev, ja
From: David Ward <david.ward@ll.mit.edu>
Date: Sun, 4 Sep 2011 09:07:21 -0400
> With the conversion of struct flowi to a union of AF-specific structs, some
> operations on the flow cache need to account for the exact size of the key.
>
> Signed-off-by: David Ward <david.ward@ll.mit.edu>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-09-16 22:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-31 16:05 [PATCH] net: Initialize entire flowi struct David Ward
2011-08-31 20:51 ` Julian Anastasov
2011-08-31 20:47 ` David Miller
2011-09-01 13:34 ` Ward, David - 0663 - MITLL
2011-09-03 7:27 ` Julian Anastasov
2011-09-04 13:07 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
2011-09-04 13:07 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
2011-09-16 22:57 ` David Miller
2011-09-04 13:07 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2011-09-04 15:34 ` Michał Mirosław
2011-09-06 2:47 ` [PATCH 0/2] Fixes to flow cache for AF-specifc flowi structs David Ward
2011-09-06 2:47 ` [PATCH 1/2] net: Align AF-specific flowi structs to long David Ward
2011-09-06 2:47 ` [PATCH 2/2] net: Handle different key sizes between address families in flow cache David Ward
2011-09-16 22:57 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).