Re: [RFC][PATCH 3/3] TCP/IP Critical socket communication mechanism
On Mer, 2005-12-14 at 01:12 -0800, Sridhar Samudrala wrote: Pass __GFP_CRITICAL flag with all allocation requests that are critical. - All allocations needed to process incoming packets are marked as CRITICAL. This includes the allocations - made by the driver to receive incoming packets - to process and send ARP packets - to create new routes for incoming packets But your user space that would add the routes is not so protected so I'm not sure this is actually a solution, more of an extended fudge. In which case I'm not clear why it is any better than the current GFP_ATOMIC approach. +#define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) Lots of hidden conditional logic on critical paths. Also sk should be in brackets so that the macro evaluation order is defined as should flags +#define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) Pointless obfuscation - 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: [RFC][PATCH 3/3] TCP/IP Critical socket communication mechanism
Alan Cox wrote: But your user space that would add the routes is not so protected so I'm not sure this is actually a solution, more of an extended fudge. Yes, there's no 100% solution -- no matter how much memory you reserve and how many paths you protect if you try hard enough you can come up with cases where it'll fail. (I'm swapping to NFS across a tun/tap interface to a custom userland SSL tunnel to a server across a BGP route...) However, if the 'extended fundge' pushes a problem from can happen, even in a very normal setup territory to only happens if you're doing something pretty weird then is it really such a bad thing? I think the cost in code complexity looks pretty reasonable. +#define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) Lots of hidden conditional logic on critical paths. How expensive is it compared to the allocation itself? +#define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) Pointless obfuscation Fully agree. -Mitch - 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: [RFC][PATCH 3/3] TCP/IP Critical socket communication mechanism
On Wed, 2005-12-14 at 11:17 +, Alan Cox wrote: On Mer, 2005-12-14 at 01:12 -0800, Sridhar Samudrala wrote: Pass __GFP_CRITICAL flag with all allocation requests that are critical. - All allocations needed to process incoming packets are marked as CRITICAL. This includes the allocations - made by the driver to receive incoming packets - to process and send ARP packets - to create new routes for incoming packets But your user space that would add the routes is not so protected so I'm not sure this is actually a solution, more of an extended fudge. In which case I'm not clear why it is any better than the current GFP_ATOMIC approach. I am not referring to routes that are added by user-space, but the allocations needed for cached routes stored in skb-dst in ip_route_input() path. +#define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) Lots of hidden conditional logic on critical paths. Also sk should be in brackets so that the macro evaluation order is defined as should flags +#define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) Pointless obfuscation The only reason i made these macros is that i would expect this to a compile time configurable option so that there is zero overhead for regular users. #ifdef CONFIG_CRIT_SOCKET #define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) #define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) #else #define SK_CRIT_ALLOC(sk, flags) flags #define CRIT_ALLOC(flags) flags #endif Thanks Sridhar - 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: [RFC][PATCH 3/3] TCP/IP Critical socket communication mechanism
Sridhar Samudrala wrote: The only reason i made these macros is that i would expect this to a compile time configurable option so that there is zero overhead for regular users. #ifdef CONFIG_CRIT_SOCKET #define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) #define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) #else #define SK_CRIT_ALLOC(sk, flags) flags #define CRIT_ALLOC(flags) flags #endif Oh, that's much simpler to achieve: #ifdef CONFIG_CRIT_SOCKET #define __GFP_CRITICAL_SOCKET __GFP_CRITICAL #else #define __GFP_CRITICAL_SOCKET 0 #endif Maybe we can get better naming here, but you get the point, I think. 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: [RFC][PATCH 3/3] TCP/IP Critical socket communication mechanism
On Wed, 2005-12-14 at 04:12 -0800, Mitchell Blank Jr wrote: Alan Cox wrote: But your user space that would add the routes is not so protected so I'm not sure this is actually a solution, more of an extended fudge. Yes, there's no 100% solution -- no matter how much memory you reserve and how many paths you protect if you try hard enough you can come up with cases where it'll fail. (I'm swapping to NFS across a tun/tap interface to a custom userland SSL tunnel to a server across a BGP route...) However, if the 'extended fundge' pushes a problem from can happen, even in a very normal setup territory to only happens if you're doing something pretty weird then is it really such a bad thing? I think the cost in code complexity looks pretty reasonable. Yes. This should work fine for cases where you need a limited number of critical allocation requests to succeed for a short period of time. +#define SK_CRIT_ALLOC(sk, flags) ((sk-sk_allocation __GFP_CRITICAL) | flags) Lots of hidden conditional logic on critical paths. How expensive is it compared to the allocation itself? Also, as i said in my other response we could make it a compile-time configurable option with zero overhead when turned off. Thanks Sridhar +#define CRIT_ALLOC(flags) (__GFP_CRITICAL | flags) Pointless obfuscation Fully agree. -Mitch - 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
