Hi Stefano,
On Fri, 28 Feb 2020, Stefano Brivio wrote:
> On Thu, 27 Feb 2020 21:37:10 +0100 (CET)
> Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx> wrote:
>
> > On Tue, 25 Feb 2020, Stefano Brivio wrote:
> >
> > > On Tue, 25 Feb 2020 21:37:45 +0100 (CET)
> > > Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx> wrote:
> > >
> > > > On Tue, 25 Feb 2020, Stefano Brivio wrote:
> > > >
> > > > > > The logic could be changed in the user rules from
> > > > > >
> > > > > > iptables -I INPUT -m set --match-set c src --bytes-gt 800 -j DROP
> > > > > >
> > > > > > to
> > > > > >
> > > > > > iptables -I INPUT -m set --match-set c src --bytes-lt 800 -j ACCEPT
> > > > > > [ otherwise DROP ]
> > > > > >
> > > > > > but of course it might be not so simple, depending on how the rules are
> > > > > > built up.
> > > > >
> > > > > Yes, it would work, unless the user actually wants to check with the
> > > > > same counter how many bytes are sent "in excess".
> > > >
> > > > You mean the counters are still updated whenever the element is matched in
> > > > the set and then one could check how many bytes were sent over the
> > > > threshold just by listing the set elements.
> > >
> > > Yes, exactly -- note that it was possible (and, I think, used) before.
> >
> > I'm still not really convinced about such a feature. Why is it useful to
> > know how many bytes would be sent over the "limit"?
>
> This is useful in case one wants different treatments for packets
> according to a number of thresholds in different rules. For example,
>
> iptables -I INPUT -m set --match-set c src --bytes-lt 100 -j noise
> iptables -I noise -m set --match-set c src --bytes-lt 20000 -j download
>
> and you want to log packets from chains 'noise' and 'download' with
> different prefixes.
What do you think about this patch?
diff --git a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
index 7545af4..6881329 100644
--- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
@@ -186,6 +186,9 @@ enum ipset_cmd_flags {
IPSET_FLAG_MAP_SKBPRIO = (1 << IPSET_FLAG_BIT_MAP_SKBPRIO),
IPSET_FLAG_BIT_MAP_SKBQUEUE = 10,
IPSET_FLAG_MAP_SKBQUEUE = (1 << IPSET_FLAG_BIT_MAP_SKBQUEUE),
+ IPSET_FLAG_BIT_UPDATE_COUNTERS_FIRST = 11,
+ IPSET_FLAG_UPDATE_COUNTERS_FIRST =
+ (1 << IPSET_FLAG_BIT_UPDATE_COUNTERS_FIRST),
IPSET_FLAG_CMD_MAX = 15,
};
diff --git a/kernel/net/netfilter/ipset/ip_set_core.c b/kernel/net/netfilter/ipset/ip_set_core.c
index 1df6536..423d0de 100644
--- a/kernel/net/netfilter/ipset/ip_set_core.c
+++ b/kernel/net/netfilter/ipset/ip_set_core.c
@@ -622,10 +622,9 @@ ip_set_add_packets(u64 packets, struct ip_set_counter *counter)
static void
ip_set_update_counter(struct ip_set_counter *counter,
- const struct ip_set_ext *ext, u32 flags)
+ const struct ip_set_ext *ext)
{
- if (ext->packets != ULLONG_MAX &&
- !(flags & IPSET_FLAG_SKIP_COUNTER_UPDATE)) {
+ if (ext->packets != ULLONG_MAX) {
ip_set_add_bytes(ext->bytes, counter);
ip_set_add_packets(ext->packets, counter);
}
@@ -649,13 +648,19 @@ ip_set_match_extensions(struct ip_set *set, const struct ip_set_ext *ext,
if (SET_WITH_COUNTER(set)) {
struct ip_set_counter *counter = ext_counter(data, set);
+ if (flags & IPSET_FLAG_UPDATE_COUNTERS_FIRST)
+ ip_set_update_counter(counter, ext);
+
if (flags & IPSET_FLAG_MATCH_COUNTERS &&
!(ip_set_match_counter(ip_set_get_packets(counter),
mext->packets, mext->packets_op) &&
ip_set_match_counter(ip_set_get_bytes(counter),
mext->bytes, mext->bytes_op)))
return false;
- ip_set_update_counter(counter, ext, flags);
+
+ if (!(flags & (IPSET_FLAG_UPDATE_COUNTERS_FIRST|
+ IPSET_FLAG_SKIP_COUNTER_UPDATE)))
+ ip_set_update_counter(counter, ext);
}
if (SET_WITH_SKBINFO(set))
ip_set_get_skbinfo(ext_skbinfo(data, set),
Then the rules above would look like
... -m set ... --update-counters-first --bytes-lt 100 -j noise
... -m set ... --update-counters-first --bytes-ge 100 -j download
> > > What I meant is really the case where "--update-counters" (or
> > > "--force-update-counters") and "! --update-counters" are both
> > > absent: I don't see any particular advantage in the current
> > > behaviour for that case.
> >
> > The counters are used just for statistical purposes: reflect the
> > packets/bytes which were let through, i.e. matched the whole "rule".
> > In that case updating the counters before the counter value matching
> > is evaluated gives false results.
>
> Well, but for that, iptables/x_tables counters are available and (as far
> as I know) typically used.
With "rules" I meant at ipset level (match element + packet/byte counters
as specified), i.e. counters for statistical purposes per set elements
level.
Best regards,
Jozsef
-
E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxx
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary
