On Thursday 19 September 2013 11:30:50 Jozsef Kadlecsik wrote: > On Thu, 19 Sep 2013, Oliver wrote: > > > On Tue, 17 Sep 2013, Oliver wrote: > > <snip> > > > > > > @@ -461,6 +462,9 @@ mtype_expire(struct ip_set *set, struct htype *h, > > > > u8 > > > > nets_length, size_t dsize)> > > > > > > > > struct mtype_elem *data; > > > > u32 i; > > > > int j; > > > > > > > > +#if IPSET_NET_COUNT > 1 > > > > + u8 k; > > > > +#endif > > > > > > Please get rid of all these #if .. [#else ...] #endif constructions, > > > except in mtype_test_cidrs. The compiler optimizes away the for loop > > > when > > > IPSET_NET_COUNT == 1. > > > > Yep, there's a couple of other places where I don't currently see a way > > around it, but anywhere that it doesn't pose a logical problem to > > existing types, I've removed it. > > Where do you think it's required, except in mtype_test_cidrs? Well actually, I've come to the realisation that it's actually required pretty much everywhere that I placed it because of the simple reason that in the existing hash types, data->cidr is not a u8 array, but it is in hash:net,net obviously. So now either we can convert all the existing hash types so that the cidr member of their struct is a single-element array, or we keep the preprocessor conditions, your choice. > > > > > + if (unlikely(!tb[IPSET_ATTR_IP] || !tb[IPSET_ATTR_IP2] || > > > > + !ip_set_optattr_netorder(tb, IPSET_ATTR_TIMEOUT) || > > > > + !ip_set_optattr_netorder(tb, IPSET_ATTR_CADT_FLAGS) || > > > > + !ip_set_optattr_netorder(tb, IPSET_ATTR_PACKETS) || > > > > + !ip_set_optattr_netorder(tb, IPSET_ATTR_BYTES))) > > > > + return -IPSET_ERR_PROTOCOL; > > > > + if (unlikely(tb[IPSET_ATTR_IP_TO])) > > > > + return -IPSET_ERR_HASH_RANGE_UNSUPPORTED; > > > > > > Check tb[IPSET_ATTR_IP2_TO] in the condition above too. > > > > This is IPv6, I thought ranges weren't even legitimate here, also, if > > this is wrong, hash:net must be too... Or does IP2_TO not mean what I > > think it means? > > IPSET_ATTR_IP2_TO carries the endpoint of the range of the second IP type > of element and ranges are not allowed in either IP type of element part > for IPv6. It is not valid for hash:net at all, but messages for > hash:net,net types may carry it. ipset itself does not allow the syntax, > but the library is free to use for anyone and therefore the kernel must > handle the case. OK, I see what you mean now, for some reason I didn't see the IPSET_ATTR_IP_TO check, obviously TO2 is needed, I will fix that before sending v2. Thanks, Oliver. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
