Message ID | alpine.DEB.2.00.1209182232460.5113@blackhole.kfki.hu |
---|---|
State | Not Applicable |
Headers | show |
On Tue, 18 Sep 2012, Jozsef Kadlecsik wrote: > I propose a small cache for inter-match communication purpose: > > diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h > index 8d674a7..f07eab2 100644 > --- a/include/linux/netfilter/x_tables.h > +++ b/include/linux/netfilter/x_tables.h > @@ -216,6 +216,9 @@ struct xt_action_param { > const void *matchinfo, *targinfo; > }; > const struct net_device *in, *out; > +#ifdef CONFIG_NETFILTER_XTABLES_CACHE > + u_int32_t cache; > +#endif Perhaps we should add it, that the end of the struct, to avoid too big ABI breakage. And I generally don't like, adding compile time optional elements in the middle of a struct, as it make its harder to cache profile and padding/aligning the struct. > int fragoff; > unsigned int thoff; > unsigned int hooknum; > @@ -223,6 +226,15 @@ struct xt_action_param { > bool hotdrop; > }; > > +enum xt_cache_owner { > + XT_CACHE_OWNER_NONE = 0, > + XT_CACHE_OWNER_IPSET = 1, > +}; > + > +#define XT_CACHE_GET_OWNER(cache) (((cache) & 0xFF000000) >> 24) > +#define XT_CACHE_SET_OWNER(cache, owner) ((cache) |= (owner) << 24) > +#define XT_CACHE_GET_VALUE(cache) ((cache) & 0x00FFFFFF) > + So, you are reserving 24 bit for data/"values". And we have 8 bits for setting an owner of this data. Thats the basic idea right? Cheers, Jesper Brouer -- ------------------------------------------------------------------- MSc. Master of Computer Science Dept. of Computer Science, University of Copenhagen Author of http://www.adsl-optimizer.dk ------------------------------------------------------------------- -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 2012-09-19 17:38, Jesper Dangaard Brouer wrote: >> +++ b/include/linux/netfilter/x_tables.h >> @@ -216,6 +216,9 @@ struct xt_action_param { >> const void *matchinfo, *targinfo; >> }; >> const struct net_device *in, *out; >> +#ifdef CONFIG_NETFILTER_XTABLES_CACHE >> + u_int32_t cache; >> +#endif > > Perhaps we should add it, that the end of the struct, to avoid too big ABI > breakage. It makes no difference where it is added. The ABI changes either way. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Sep 2012, Jesper Dangaard Brouer wrote: > On Tue, 18 Sep 2012, Jozsef Kadlecsik wrote: > > > I propose a small cache for inter-match communication purpose: > > > > diff --git a/include/linux/netfilter/x_tables.h > > b/include/linux/netfilter/x_tables.h > > index 8d674a7..f07eab2 100644 > > --- a/include/linux/netfilter/x_tables.h > > +++ b/include/linux/netfilter/x_tables.h > > @@ -216,6 +216,9 @@ struct xt_action_param { > > const void *matchinfo, *targinfo; > > }; > > const struct net_device *in, *out; > > +#ifdef CONFIG_NETFILTER_XTABLES_CACHE > > + u_int32_t cache; > > +#endif > > Perhaps we should add it, that the end of the struct, to avoid too big ABI > breakage. And I generally don't like, adding compile time optional elements > in the middle of a struct, as it make its harder to cache profile and > padding/aligning the struct. I did not want to create a hole that's why I put the new structure element after *out. The optional compiling in is just a suggestion :-) > > int fragoff; > > unsigned int thoff; > > unsigned int hooknum; > > @@ -223,6 +226,15 @@ struct xt_action_param { > > bool hotdrop; > > }; > > > > > +enum xt_cache_owner { > > + XT_CACHE_OWNER_NONE = 0, > > + XT_CACHE_OWNER_IPSET = 1, > > +}; > > + > > +#define XT_CACHE_GET_OWNER(cache) (((cache) & 0xFF000000) >> 24) > > +#define XT_CACHE_SET_OWNER(cache, owner) ((cache) |= (owner) << 24) > > +#define XT_CACHE_GET_VALUE(cache) ((cache) & 0x00FFFFFF) > > + > > So, you are reserving 24 bit for data/"values". And we have 8 bits for > setting an owner of this data. Thats the basic idea right? Yes, but we could reserve less bits, say 6, 5 or even 4 for the owner and so giving more space to the value. Of course the cache is too small to hold a pointer, a match can store internal state flags there. For the "set" match that means the set identifier (16 bits) and the match flags together with the match result (8 bits). Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jozsef, On Tue, Sep 18, 2012 at 11:01:34PM +0200, Jozsef Kadlecsik wrote: > Hi, > > I propose a small cache for inter-match communication purpose: > > diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h > index 8d674a7..f07eab2 100644 > --- a/include/linux/netfilter/x_tables.h > +++ b/include/linux/netfilter/x_tables.h > @@ -216,6 +216,9 @@ struct xt_action_param { > const void *matchinfo, *targinfo; > }; > const struct net_device *in, *out; > +#ifdef CONFIG_NETFILTER_XTABLES_CACHE > + u_int32_t cache; > +#endif I think you can implement this by means of one per-CPU cache inside the xt_set match. Check the old per-CPU event cache in net/netfilter/nf_conntrack_ecache.c. We used to have something similar. I'd prefer this approach rather than one change in xtables for this. It still seems to me too specific of your xt_set extensions. It will remain internal of xt_set match, but we can revisit this later on to generalize it if it becomes interesting for more matches. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 24 Sep 2012, Pablo Neira Ayuso wrote: > On Tue, Sep 18, 2012 at 11:01:34PM +0200, Jozsef Kadlecsik wrote: > > > > I propose a small cache for inter-match communication purpose: > > > > diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h > > index 8d674a7..f07eab2 100644 > > --- a/include/linux/netfilter/x_tables.h > > +++ b/include/linux/netfilter/x_tables.h > > @@ -216,6 +216,9 @@ struct xt_action_param { > > const void *matchinfo, *targinfo; > > }; > > const struct net_device *in, *out; > > +#ifdef CONFIG_NETFILTER_XTABLES_CACHE > > + u_int32_t cache; > > +#endif > > I think you can implement this by means of one per-CPU cache inside > the xt_set match. > > Check the old per-CPU event cache in net/netfilter/nf_conntrack_ecache.c. > We used to have something similar. That's a good idea! I'd prefer something encapsulated inside xt_set as well, I'm going to check this possibility. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index 8d674a7..f07eab2 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -216,6 +216,9 @@ struct xt_action_param { const void *matchinfo, *targinfo; }; const struct net_device *in, *out; +#ifdef CONFIG_NETFILTER_XTABLES_CACHE + u_int32_t cache; +#endif int fragoff; unsigned int thoff; unsigned int hooknum; @@ -223,6 +226,15 @@ struct xt_action_param { bool hotdrop; }; +enum xt_cache_owner { + XT_CACHE_OWNER_NONE = 0, + XT_CACHE_OWNER_IPSET = 1, +}; + +#define XT_CACHE_GET_OWNER(cache) (((cache) & 0xFF000000) >> 24) +#define XT_CACHE_SET_OWNER(cache, owner) ((cache) |= (owner) << 24) +#define XT_CACHE_GET_VALUE(cache) ((cache) & 0x00FFFFFF) + /** * struct xt_mtchk_param - parameters for match extensions' * checkentry functions