Message ID | 1365857474-4943-2-git-send-email-kadlec@blackhole.kfki.hu |
---|---|
State | Accepted |
Headers | show |
Hi Jozsef, On Sat, Apr 13, 2013 at 02:51:14PM +0200, Jozsef Kadlecsik wrote: > The type when timeout support was enabled, could not list all elements, > just the first ones which could fit into one netlink message: it just > did not continue listing after the first message. > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > --- > net/netfilter/ipset/ip_set_bitmap_ipmac.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > index 0f92dc2..d7df6ac 100644 > --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > @@ -339,7 +339,11 @@ bitmap_ipmac_tlist(const struct ip_set *set, > nla_put_failure: > nla_nest_cancel(skb, nested); > ipset_nest_end(skb, atd); I think this ipset_nest_end should be after the id == first checking. It doesn't make sense for the -EMSGSIZE case. BTW, in the first message, where `first' is unset, id will never equal first and you will always return success even if you could not add one single nested attribute into the message. > - return -EMSGSIZE; > + if (unlikely(id == first)) { > + cb->args[2] = 0; > + return -EMSGSIZE; > + } > + return 0; Regards. -- 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 Pablo, On Tue, 16 Apr 2013, Pablo Neira Ayuso wrote: > On Sat, Apr 13, 2013 at 02:51:14PM +0200, Jozsef Kadlecsik wrote: > > The type when timeout support was enabled, could not list all elements, > > just the first ones which could fit into one netlink message: it just > > did not continue listing after the first message. > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > --- > > net/netfilter/ipset/ip_set_bitmap_ipmac.c | 6 +++++- > > 1 files changed, 5 insertions(+), 1 deletions(-) > > > > diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > index 0f92dc2..d7df6ac 100644 > > --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > @@ -339,7 +339,11 @@ bitmap_ipmac_tlist(const struct ip_set *set, > > nla_put_failure: > > nla_nest_cancel(skb, nested); > > ipset_nest_end(skb, atd); > > I think this ipset_nest_end should be after the id == first checking. > It doesn't make sense for the -EMSGSIZE case. Yes, that could be moved there - candidate for the nf-next tree? > BTW, in the first message, where `first' is unset, id will never equal > first and you will always return success even if you could not add one > single nested attribute into the message. "first" is always initialized: it's either zero (the id of the first entry) or the id of the next one where listing must be continued. 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, Apr 16, 2013 at 09:16:18PM +0200, Jozsef Kadlecsik wrote: > Hi Pablo, > > On Tue, 16 Apr 2013, Pablo Neira Ayuso wrote: > > > On Sat, Apr 13, 2013 at 02:51:14PM +0200, Jozsef Kadlecsik wrote: > > > The type when timeout support was enabled, could not list all elements, > > > just the first ones which could fit into one netlink message: it just > > > did not continue listing after the first message. > > > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > > --- > > > net/netfilter/ipset/ip_set_bitmap_ipmac.c | 6 +++++- > > > 1 files changed, 5 insertions(+), 1 deletions(-) > > > > > > diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > > index 0f92dc2..d7df6ac 100644 > > > --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > > +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > > @@ -339,7 +339,11 @@ bitmap_ipmac_tlist(const struct ip_set *set, > > > nla_put_failure: > > > nla_nest_cancel(skb, nested); > > > ipset_nest_end(skb, atd); > > > > I think this ipset_nest_end should be after the id == first checking. > > It doesn't make sense for the -EMSGSIZE case. > > Yes, that could be moved there - candidate for the nf-next tree? Yes. This is not critical, send me a follow up this in a follow up patch for nf-next. > > BTW, in the first message, where `first' is unset, id will never equal > > first and you will always return success even if you could not add one > > single nested attribute into the message. > > "first" is always initialized: it's either zero (the id of the first > entry) or the id of the next one where listing must be continued. I see, that's OK. I have applied this patch. Thanks. -- 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/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c index 0f92dc2..d7df6ac 100644 --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c @@ -339,7 +339,11 @@ bitmap_ipmac_tlist(const struct ip_set *set, nla_put_failure: nla_nest_cancel(skb, nested); ipset_nest_end(skb, atd); - return -EMSGSIZE; + if (unlikely(id == first)) { + cb->args[2] = 0; + return -EMSGSIZE; + } + return 0; } static int
The type when timeout support was enabled, could not list all elements, just the first ones which could fit into one netlink message: it just did not continue listing after the first message. Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> --- net/netfilter/ipset/ip_set_bitmap_ipmac.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)