* [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 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 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 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
* 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
* [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 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).