Re: [PATCH] [NET] reduce sizeof(struct flow)
From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 07:42:17 +0200 How many people are using DECNET and want to pay the price of this 20 bytes dnports structure ? I bet you could make that cost get hidden by careful rearrangement of the struct flow, or adjustment of the implementation. BTW, I see assignments to these fields, and as a specific example uli_u.dnports.objnamel but no use of it. Perhaps much of dnports can even be deleted outright. :-) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
Hi, On Tue, Oct 17, 2006 at 11:53:36PM -0700, David Miller wrote: From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 07:42:17 +0200 How many people are using DECNET and want to pay the price of this 20 bytes dnports structure ? Point taken :-) Eric, you also need to add a || defined(CONFIG_DECNET_MODULE) I think in your patch, if you want to make this optional. I bet you could make that cost get hidden by careful rearrangement of the struct flow, or adjustment of the implementation. BTW, I see assignments to these fields, and as a specific example uli_u.dnports.objnamel but no use of it. Perhaps much of dnports can even be deleted outright. :-) - Its not used at the moment[*], but would be required for any kind of flow tracking. The objnum field, could be folded into the objname field I guess on the basis that objnamel == 0 means objname[0] represents the objnum, but that doesn't really buy much. Looking at the rearrangement option, and the relative lengths of ipv6 and DECnet node addresses, dn_u is a lot smaller than ip6_u and thus the obj[num|name|namel] fields could be moved into that structure. Even after doing this, dn_u would still be shorter than ip6_u, although 12 bytes longer than ip4_u (if my counting is correct). Is that an acceptable solution? [*] By which I mean, as you've already alluded to, that its set up correctly but not currently read/tested by anything yet. Steve. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
On Wednesday 18 October 2006 10:20, Steven Whitehouse wrote: Hi, On Tue, Oct 17, 2006 at 11:53:36PM -0700, David Miller wrote: From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 07:42:17 +0200 How many people are using DECNET and want to pay the price of this 20 bytes dnports structure ? Point taken :-) Eric, you also need to add a || defined(CONFIG_DECNET_MODULE) I think in your patch, if you want to make this optional. Yes, thank you :) I bet you could make that cost get hidden by careful rearrangement of the struct flow, or adjustment of the implementation. BTW, I see assignments to these fields, and as a specific example uli_u.dnports.objnamel but no use of it. Perhaps much of dnports can even be deleted outright. :-) - Its not used at the moment[*], but would be required for any kind of flow tracking. The objnum field, could be folded into the objname field I guess on the basis that objnamel == 0 means objname[0] represents the objnum, but that doesn't really buy much. Well, as I privately said to David, objnamel is used, at least on 2.6.18 tree ( net/decnet/dn_route.c function compare_keys() memcmp() will read this field and check before comparing objname[] So it is set by dn_sk_ports_copy(), and used by compare_keys() Looking at the rearrangement option, and the relative lengths of ipv6 and DECnet node addresses, dn_u is a lot smaller than ip6_u and thus the obj[num|name|namel] fields could be moved into that structure. Even after doing this, dn_u would still be shorter than ip6_u, although 12 bytes longer than ip4_u (if my counting is correct). Is that an acceptable solution? Well, most of my machines dont use IPV6 nor DECNET :) [*] By which I mean, as you've already alluded to, that its set up correctly but not currently read/tested by anything yet. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
Hi David, David Miller schrieb: I don't like these kinds of patches because %99 of people will never ever realize the savings because distribution vendors will always, unlaterally, enable everything. People producing Linux Appliances DO compile their own kernels. And some distribution vendors are still at 2.6.8-$WHACKY_PATCHES or similiar, which is not usable for many things (e.g. IPsec, SIP behind NAT etc.). But I agree that workings towards smaller size in make allyesconfig is much better :-) Regards Ingo Oeser - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
Hi, Its not used at the moment[*], but would be required for any kind of flow tracking. The objnum field, could be folded into the objname field I guess on the basis that objnamel == 0 means objname[0] represents the objnum, but that doesn't really buy much. Well, as I privately said to David, objnamel is used, at least on 2.6.18 tree ( net/decnet/dn_route.c function compare_keys() memcmp() will read this field and check before comparing objname[] So it is set by dn_sk_ports_copy(), and used by compare_keys() It is set by dn_sk_ports_copy() as you say, but the obj[num|name|namel] fields are not used as a key in compare_keys() at the moment, although all the other fields are used there. Since the recent bug fix to this area of the code (memcmp was comparing uninitialised padding) its easier to see what is being compared. In fact I suspect the reason that the obj fields were not put in the dn_u where they'd take up a lot less room was because the memcmp covered only dn_u and not the rest of the structure. It was a while ago that I wrote this and my memory is failing me as to the exact reason I did it like that. Looking at the rearrangement option, and the relative lengths of ipv6 and DECnet node addresses, dn_u is a lot smaller than ip6_u and thus the obj[num|name|namel] fields could be moved into that structure. Even after doing this, dn_u would still be shorter than ip6_u, although 12 bytes longer than ip4_u (if my counting is correct). Is that an acceptable solution? Well, most of my machines dont use IPV6 nor DECNET :) Its still an improvement over the current situation, even though it might be (probably is) possible to improve it further, Steve. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
On Wednesday 18 October 2006 14:42, Steven Whitehouse wrote: Hi, Its not used at the moment[*], but would be required for any kind of flow tracking. The objnum field, could be folded into the objname field I guess on the basis that objnamel == 0 means objname[0] represents the objnum, but that doesn't really buy much. Well, as I privately said to David, objnamel is used, at least on 2.6.18 tree ( net/decnet/dn_route.c function compare_keys() memcmp() will read this field and check before comparing objname[] So it is set by dn_sk_ports_copy(), and used by compare_keys() It is set by dn_sk_ports_copy() as you say, but the obj[num|name|namel] fields are not used as a key in compare_keys() at the moment, although all the other fields are used there. Since the recent bug fix to this area of the code (memcmp was comparing uninitialised padding) its easier to see what is being compared. Oh, I see, you are referring to 2.6.19-rc2, I was referring to 2.6.18 (what I call current linux version) In fact I suspect the reason that the obj fields were not put in the dn_u where they'd take up a lot less room was because the memcmp covered only dn_u and not the rest of the structure. It was a while ago that I wrote this and my memory is failing me as to the exact reason I did it like that. Looking at the rearrangement option, and the relative lengths of ipv6 and DECnet node addresses, dn_u is a lot smaller than ip6_u and thus the obj[num|name|namel] fields could be moved into that structure. Even after doing this, dn_u would still be shorter than ip6_u, although 12 bytes longer than ip4_u (if my counting is correct). Is that an acceptable solution? Well, most of my machines dont use IPV6 nor DECNET :) Its still an improvement over the current situation, even though it might be (probably is) possible to improve it further, OK then. Even on a distro kernel (allyesconfig), size(flow) would shrink by 20 bytes. Thank you. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 15:32:22 +0200 OK then. Even on a distro kernel (allyesconfig), size(flow) would shrink by 20 bytes. For the time being, why don't we just kill off the unused dnports bits entirely. Once the support code to use those bits is written, we can investigate intelligent ways to pack them back into struct flow, ok? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
David Miller a écrit : From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 15:32:22 +0200 OK then. Even on a distro kernel (allyesconfig), size(flow) would shrink by 20 bytes. For the time being, why don't we just kill off the unused dnports bits entirely. Once the support code to use those bits is written, we can investigate intelligent ways to pack them back into struct flow, ok? OK I will prepare a patch if Steve doesnt beat me :) Eric - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [NET] reduce sizeof(struct flow)
Hi David
Each route entry includes a 'struct flow'. This structure has a current size
of 80 bytes. This patch makes a size reduction depending on
CONFIG_IPV6/CONFIG_IPV6_MODULE/CONFIG_DECNET/CONFIG_IP_ROUTE_FWMARK
For a platform doing IPV4 only, the new size is 36 bytes (instead of 80)
As many routers are base on PIII (L1_CACHE_SIZE=32), this saves one cache line
per rtable entry.
Thank you
Signed-off-by: Eric Dumazet [EMAIL PROTECTED]
--- linux-2.6.19-rc2/include/net/flow.h 2006-10-18 06:03:08.0 +0200
+++ linux-2.6.19-rc2-ed/include/net/flow.h 2006-10-18 06:56:37.0
+0200
@@ -18,17 +18,21 @@
struct {
__be32 daddr;
__be32 saddr;
+#if defined(CONFIG_IP_ROUTE_FWMARK)
__u32 fwmark;
+#endif
__u8tos;
__u8scope;
} ip4_u;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct {
struct in6_addr daddr;
struct in6_addr saddr;
__u32 fwmark;
__u32 flowlabel;
} ip6_u;
+#endif
struct {
__le16 daddr;
@@ -65,6 +69,7 @@
__u8code;
} icmpt;
+#if defined(CONFIG_DECNET)
struct {
__le16 sport;
__le16 dport;
@@ -72,6 +77,7 @@
__u8objnamel; /* Not 16 bits since max val is 16 */
__u8objname[16]; /* Not zero terminated */
} dnports;
+#endif
__be32 spi;
--- linux-2.6.19-rc2/include/net/xfrm.h 2006-10-18 06:21:19.0 +0200
+++ linux-2.6.19-rc2-ed/include/net/xfrm.h 2006-10-18 06:53:41.0
+0200
@@ -517,6 +517,7 @@
(fl-oif == sel-ifindex || !sel-ifindex);
}
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int
__xfrm6_selector_match(struct xfrm_selector *sel, struct flowi *fl)
{
@@ -527,6 +528,7 @@
(fl-proto == sel-proto || !sel-proto)
(fl-oif == sel-ifindex || !sel-ifindex);
}
+#endif
static inline int
xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl,
@@ -535,8 +537,10 @@
switch (family) {
case AF_INET:
return __xfrm4_selector_match(sel, fl);
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return __xfrm6_selector_match(sel, fl);
+#endif
}
return 0;
}
@@ -577,7 +581,9 @@
struct xfrm_dst *next;
struct dst_entrydst;
struct rtable rt;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct rt6_info rt6;
+#endif
} u;
struct dst_entry *route;
u32 genid;
@@ -650,12 +656,14 @@
tmpl-saddr.a4 != x-props.saddr.a4);
}
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int
__xfrm6_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x)
{
return (!ipv6_addr_any((struct in6_addr*)tmpl-saddr)
ipv6_addr_cmp((struct in6_addr *)tmpl-saddr, (struct
in6_addr*)x-props.saddr));
}
+#endif
static inline int
xfrm_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x, unsigned
short family)
@@ -663,8 +671,10 @@
switch (family) {
case AF_INET:
return __xfrm4_state_addr_cmp(tmpl, x);
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return __xfrm6_state_addr_cmp(tmpl, x);
+#endif
}
return !0;
}
@@ -762,8 +772,10 @@
switch (family){
case AF_INET:
return (xfrm_address_t *)fl-fl4_dst;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return (xfrm_address_t *)fl-fl6_dst;
+#endif
}
return NULL;
}
@@ -774,8 +786,10 @@
switch (family){
case AF_INET:
return (xfrm_address_t *)fl-fl4_src;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return (xfrm_address_t *)fl-fl6_src;
+#endif
}
return NULL;
}
@@ -790,6 +804,7 @@
return 0;
}
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static __inline__ int
__xfrm6_state_addr_check(struct xfrm_state *x,
xfrm_address_t *daddr, xfrm_address_t *saddr)
@@ -801,6 +816,7 @@
return 1;
return 0;
}
+#endif
static __inline__ int
xfrm_state_addr_check(struct xfrm_state *x,
@@ -810,8 +826,10 @@
switch (family) {
Re: [PATCH] [NET] reduce sizeof(struct flow)
In article [EMAIL PROTECTED] (at Wed, 18 Oct 2006 07:08:07 +0200), Eric
Dumazet [EMAIL PROTECTED] says:
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct {
struct in6_addr daddr;
struct in6_addr saddr;
#ifdef CONFIG_IPV6_ROUTE_FWMARK
__u32 fwmark;
#endif
__u32 flowlabel;
} ip6_u;
+#endif
struct {
__le16 daddr;
--yoshfuji
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 07:08:07 +0200 Each route entry includes a 'struct flow'. This structure has a current size of 80 bytes. This patch makes a size reduction depending on CONFIG_IPV6/CONFIG_IPV6_MODULE/CONFIG_DECNET/CONFIG_IP_ROUTE_FWMARK For a platform doing IPV4 only, the new size is 36 bytes (instead of 80) As many routers are base on PIII (L1_CACHE_SIZE=32), this saves one cache line per rtable entry. I don't like these kinds of patches because %99 of people will never ever realize the savings because distribution vendors will always, unlaterally, enable everything. For example, I'm still pending to cut the patch that kills the struct flow from inet_sock's cork area since it really isn't needed. I'd rather do things that get rid of the size in a way that benefits everyone, regardless of kernel config. I also would totally support a patch that got rid of all the config ifdefs used in struct sk_buff. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [NET] reduce sizeof(struct flow)
YOSHIFUJI Hideaki / a écrit :
In article [EMAIL PROTECTED] (at Wed, 18 Oct 2006 07:08:07 +0200), Eric Dumazet
[EMAIL PROTECTED] says:
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct {
struct in6_addr daddr;
struct in6_addr saddr;
#ifdef CONFIG_IPV6_ROUTE_FWMARK
__u32 fwmark;
#endif
__u32 flowlabel;
} ip6_u;
+#endif
struct {
__le16 daddr;
--yoshfuji
Thank you Yoshfuji
New patch version follows :
Each route entry includes a 'struct flow'. This structure has a current size
of 80 bytes. This patch makes a size reduction depending on
CONFIG_IPV6/CONFIG_IPV6_MODULE/CONFIG_DECNET/CONFIG_IP_ROUTE_FWMARK/CONFIG_IPV6_ROUTE_FWMARK
For a platform doing IPV4 only, the new size is 36 bytes (instead of 80)
As many routers are base on PIII (L1_CACHE_SIZE=32), this saves one cache line
per rtable entry.
Signed-off-by: Eric Dumazet [EMAIL PROTECTED]
--- linux-2.6.19-rc2/include/net/flow.h 2006-10-18 06:03:08.0 +0200
+++ linux-2.6.19-rc2-ed/include/net/flow.h 2006-10-18 07:26:59.0
+0200
@@ -18,17 +18,23 @@
struct {
__be32 daddr;
__be32 saddr;
+#if defined(CONFIG_IP_ROUTE_FWMARK)
__u32 fwmark;
+#endif
__u8tos;
__u8scope;
} ip4_u;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct {
struct in6_addr daddr;
struct in6_addr saddr;
+#if defined(CONFIG_IPV6_ROUTE_FWMARK)
__u32 fwmark;
+#endif
__u32 flowlabel;
} ip6_u;
+#endif
struct {
__le16 daddr;
@@ -65,6 +71,7 @@
__u8code;
} icmpt;
+#if defined(CONFIG_DECNET)
struct {
__le16 sport;
__le16 dport;
@@ -72,6 +79,7 @@
__u8objnamel; /* Not 16 bits since max val is 16 */
__u8objname[16]; /* Not zero terminated */
} dnports;
+#endif
__be32 spi;
--- linux-2.6.19-rc2/include/net/xfrm.h 2006-10-18 06:21:19.0 +0200
+++ linux-2.6.19-rc2-ed/include/net/xfrm.h 2006-10-18 06:53:41.0
+0200
@@ -517,6 +517,7 @@
(fl-oif == sel-ifindex || !sel-ifindex);
}
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int
__xfrm6_selector_match(struct xfrm_selector *sel, struct flowi *fl)
{
@@ -527,6 +528,7 @@
(fl-proto == sel-proto || !sel-proto)
(fl-oif == sel-ifindex || !sel-ifindex);
}
+#endif
static inline int
xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl,
@@ -535,8 +537,10 @@
switch (family) {
case AF_INET:
return __xfrm4_selector_match(sel, fl);
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return __xfrm6_selector_match(sel, fl);
+#endif
}
return 0;
}
@@ -577,7 +581,9 @@
struct xfrm_dst *next;
struct dst_entrydst;
struct rtable rt;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
struct rt6_info rt6;
+#endif
} u;
struct dst_entry *route;
u32 genid;
@@ -650,12 +656,14 @@
tmpl-saddr.a4 != x-props.saddr.a4);
}
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int
__xfrm6_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x)
{
return (!ipv6_addr_any((struct in6_addr*)tmpl-saddr)
ipv6_addr_cmp((struct in6_addr *)tmpl-saddr, (struct
in6_addr*)x-props.saddr));
}
+#endif
static inline int
xfrm_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x, unsigned
short family)
@@ -663,8 +671,10 @@
switch (family) {
case AF_INET:
return __xfrm4_state_addr_cmp(tmpl, x);
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return __xfrm6_state_addr_cmp(tmpl, x);
+#endif
}
return !0;
}
@@ -762,8 +772,10 @@
switch (family){
case AF_INET:
return (xfrm_address_t *)fl-fl4_dst;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
return (xfrm_address_t *)fl-fl6_dst;
+#endif
}
return NULL;
}
@@
Re: [PATCH] [NET] reduce sizeof(struct flow)
David Miller a écrit : From: Eric Dumazet [EMAIL PROTECTED] Date: Wed, 18 Oct 2006 07:08:07 +0200 Each route entry includes a 'struct flow'. This structure has a current size of 80 bytes. This patch makes a size reduction depending on CONFIG_IPV6/CONFIG_IPV6_MODULE/CONFIG_DECNET/CONFIG_IP_ROUTE_FWMARK For a platform doing IPV4 only, the new size is 36 bytes (instead of 80) As many routers are base on PIII (L1_CACHE_SIZE=32), this saves one cache line per rtable entry. I don't like these kinds of patches because %99 of people will never ever realize the savings because distribution vendors will always, unlaterally, enable everything. For example, I'm still pending to cut the patch that kills the struct flow from inet_sock's cork area since it really isn't needed. I'd rather do things that get rid of the size in a way that benefits everyone, regardless of kernel config. I also would totally support a patch that got rid of all the config ifdefs used in struct sk_buff. I agree with you David (I dont like these patches as well), but I know a lot of sysadmins who recompile themselves their kernels because they have old machines and/or want optimized kernels, not 'big bloated kernels' from distro guys running the very latest intel/amd bomb. As long the #ifdefs are in .h files, I do think the pain is small. Fact is linux has many CONFIG_ things, and as many possible binary versions for a single linux-2.6.XX version as atoms in the universe... A distro cannot provides all version and just provide the largest possible one, containing many obsolete/funny features. How many people are using DECNET and want to pay the price of this 20 bytes dnports structure ? Eric - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
