| Message ID | 20191116213218.14698-1-phil@nwl.cc |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [nft] tests/py: Set a fixed timezone in nft-test.py | expand |
I had a hard time getting time zones right. Thank you. Acked-by: Ander Juaristi <a@juaristi.eus> El 2019-11-16 22:32, Phil Sutter escribió: > Payload generated for 'meta time' matches depends on host's timezone > and > DST setting. To produce constant output, set a fixed timezone in > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > the remaining two tests. > > Fixes: 0518ea3f70d8c ("tests: add meta time test cases") > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > tests/py/any/meta.t | 2 +- > tests/py/any/meta.t.payload | 2 +- > tests/py/nft-test.py | 1 + > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tests/py/any/meta.t b/tests/py/any/meta.t > index 86e5d258605dc..327f973f1bd5a 100644 > --- a/tests/py/any/meta.t > +++ b/tests/py/any/meta.t > @@ -205,7 +205,7 @@ meta random eq 1;ok;meta random 1 > meta random gt 1000000;ok;meta random > 1000000 > > meta time "1970-05-23 21:07:14" drop;ok > -meta time 12341234 drop;ok;meta time "1970-05-23 21:07:14" drop > +meta time 12341234 drop;ok;meta time "1970-05-23 22:07:14" drop > meta time "2019-06-21 17:00:00" drop;ok > meta time "2019-07-01 00:00:00" drop;ok > meta time "2019-07-01 00:01:00" drop;ok > diff --git a/tests/py/any/meta.t.payload b/tests/py/any/meta.t.payload > index 402caae5cad8c..486d7aa566ea3 100644 > --- a/tests/py/any/meta.t.payload > +++ b/tests/py/any/meta.t.payload > @@ -1050,7 +1050,7 @@ ip test-ip4 input > # meta time "1970-05-23 21:07:14" drop > ip meta-test input > [ meta load time => reg 1 ] > - [ cmp eq reg 1 0x74a8f400 0x002bd849 ] > + [ cmp eq reg 1 0x43f05400 0x002bd503 ] > [ immediate reg 0 drop ] > > # meta time 12341234 drop > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py > index ce42b5ddb1cca..6edca3c6a5a2f 100755 > --- a/tests/py/nft-test.py > +++ b/tests/py/nft-test.py > @@ -24,6 +24,7 @@ import tempfile > > TESTS_PATH = os.path.dirname(os.path.abspath(__file__)) > sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/')) > +os.environ['TZ'] = 'UTC-2' > > from nftables import Nftables
Hi Phil, On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > Payload generated for 'meta time' matches depends on host's timezone and > DST setting. To produce constant output, set a fixed timezone in > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > the remaining two tests. This means that the ruleset listing for the user changes when daylight saving occurs, right? Just like it happened to our tests. Thanks.
Hi Pablo, On Mon, Nov 18, 2019 at 07:34:59PM +0100, Pablo Neira Ayuso wrote: > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > > Payload generated for 'meta time' matches depends on host's timezone and > > DST setting. To produce constant output, set a fixed timezone in > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > > the remaining two tests. > > This means that the ruleset listing for the user changes when daylight > saving occurs, right? Just like it happened to our tests. I think UTC-2 is a fixed offset to a time that doesn't change. During DST in Europe, our timezone changes from CET to CEST, or from UTC+1 to UTC+2. So by specifying the same TZ no matter if DST applies or not, the offset added to the current time should always remain the same. Cheers, Phil
El 2019-11-18 19:34, Pablo Neira Ayuso escribió: > Hi Phil, > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: >> Payload generated for 'meta time' matches depends on host's timezone >> and >> DST setting. To produce constant output, set a fixed timezone in >> nft-test.py. Choose UTC-2 since most payloads are correct then, adjust >> the remaining two tests. > > This means that the ruleset listing for the user changes when daylight > saving occurs, right? Just like it happened to our tests. It shouldn't, as the date is converted to a timestamp that doesn't take DST into account (using timegm(3), which is Linux-specific). The problem is that payloads are hard-coded in the tests. Correct me if I'm missing something. > > Thanks.
On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote: > El 2019-11-18 19:34, Pablo Neira Ayuso escribió: > > Hi Phil, > > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > > > Payload generated for 'meta time' matches depends on host's timezone > > > and > > > DST setting. To produce constant output, set a fixed timezone in > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > > > the remaining two tests. > > > > This means that the ruleset listing for the user changes when daylight > > saving occurs, right? Just like it happened to our tests. > > It shouldn't, as the date is converted to a timestamp that doesn't take DST > into account (using timegm(3), which is Linux-specific). > > The problem is that payloads are hard-coded in the tests. > > Correct me if I'm missing something. I see, so it's just the _snprintf() function in the library. I remember we found another problem with these on big endian, it would be probably to move them to libnftables at some point. Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Hi, On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote: > On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote: > > El 2019-11-18 19:34, Pablo Neira Ayuso escribió: > > > Hi Phil, > > > > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > > > > Payload generated for 'meta time' matches depends on host's timezone > > > > and > > > > DST setting. To produce constant output, set a fixed timezone in > > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > > > > the remaining two tests. > > > > > > This means that the ruleset listing for the user changes when daylight > > > saving occurs, right? Just like it happened to our tests. > > > > It shouldn't, as the date is converted to a timestamp that doesn't take DST > > into account (using timegm(3), which is Linux-specific). > > > > The problem is that payloads are hard-coded in the tests. > > > > Correct me if I'm missing something. > > I see, so it's just the _snprintf() function in the library. I > remember we found another problem with these on big endian, it would > be probably to move them to libnftables at some point. > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> Ah, now that I reread your question, I finally got it. And you're right: If DST occurs, time values will change. This is clear from looking at hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff added to it. Here, this is either 3600 or 7200 depending on whether DST is active or not. The other alternative would be to make kernel DST-aware, I don't think that's the case. Cheers, Phil
On Tue, Nov 19, 2019 at 11:20:23PM +0100, Phil Sutter wrote: > Hi, > > On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote: > > On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote: > > > El 2019-11-18 19:34, Pablo Neira Ayuso escribió: > > > > Hi Phil, > > > > > > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > > > > > Payload generated for 'meta time' matches depends on host's timezone > > > > > and > > > > > DST setting. To produce constant output, set a fixed timezone in > > > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > > > > > the remaining two tests. > > > > > > > > This means that the ruleset listing for the user changes when daylight > > > > saving occurs, right? Just like it happened to our tests. > > > > > > It shouldn't, as the date is converted to a timestamp that doesn't take DST > > > into account (using timegm(3), which is Linux-specific). > > > > > > The problem is that payloads are hard-coded in the tests. > > > > > > Correct me if I'm missing something. > > > > I see, so it's just the _snprintf() function in the library. I > > remember we found another problem with these on big endian, it would > > be probably to move them to libnftables at some point. > > > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> > > Ah, now that I reread your question, I finally got it. And you're right: > If DST occurs, time values will change. This is clear from looking at > hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff > added to it. Here, this is either 3600 or 7200 depending on whether DST > is active or not. > > The other alternative would be to make kernel DST-aware, I don't think > that's the case. Hm, so the ruleset listing changes after DST. If the user specifies the time according to the userspace timezone, then ruleset listing should not change? I'm still trying to understand if this is a real problem.
Hi Pablo, On Tue, Nov 19, 2019 at 11:33:18PM +0100, Pablo Neira Ayuso wrote: > On Tue, Nov 19, 2019 at 11:20:23PM +0100, Phil Sutter wrote: > > Hi, > > > > On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote: > > > On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote: > > > > El 2019-11-18 19:34, Pablo Neira Ayuso escribió: > > > > > Hi Phil, > > > > > > > > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote: > > > > > > Payload generated for 'meta time' matches depends on host's timezone > > > > > > and > > > > > > DST setting. To produce constant output, set a fixed timezone in > > > > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust > > > > > > the remaining two tests. > > > > > > > > > > This means that the ruleset listing for the user changes when daylight > > > > > saving occurs, right? Just like it happened to our tests. > > > > > > > > It shouldn't, as the date is converted to a timestamp that doesn't take DST > > > > into account (using timegm(3), which is Linux-specific). > > > > > > > > The problem is that payloads are hard-coded in the tests. > > > > > > > > Correct me if I'm missing something. > > > > > > I see, so it's just the _snprintf() function in the library. I > > > remember we found another problem with these on big endian, it would > > > be probably to move them to libnftables at some point. > > > > > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > > Ah, now that I reread your question, I finally got it. And you're right: > > If DST occurs, time values will change. This is clear from looking at > > hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff > > added to it. Here, this is either 3600 or 7200 depending on whether DST > > is active or not. > > > > The other alternative would be to make kernel DST-aware, I don't think > > that's the case. > > Hm, so the ruleset listing changes after DST. Yes, but only because payload (and hence kernel data) remains the same. > If the user specifies the time according to the userspace timezone, > then ruleset listing should not change? > > I'm still trying to understand if this is a real problem. IMHO, it's a controversy of whether we want to make the time match respect DST changes or not: It should, because when I restrict internet access to my kids after 8pm, I don't want to touch my firewall's ruleset twice a year. It needs not, because upon the next reboot things are correct again, anyway. It should not because otherwise once a year there are two spooky hours in which things run twice that should never ever run more than once per night. Apart from that, description of --kerneltz in iptables-extensions(8) speaks for itself. So long story short, it is always a problem no matter how it is solved. :( Cheers, Phil
diff --git a/tests/py/any/meta.t b/tests/py/any/meta.t index 86e5d258605dc..327f973f1bd5a 100644 --- a/tests/py/any/meta.t +++ b/tests/py/any/meta.t @@ -205,7 +205,7 @@ meta random eq 1;ok;meta random 1 meta random gt 1000000;ok;meta random > 1000000 meta time "1970-05-23 21:07:14" drop;ok -meta time 12341234 drop;ok;meta time "1970-05-23 21:07:14" drop +meta time 12341234 drop;ok;meta time "1970-05-23 22:07:14" drop meta time "2019-06-21 17:00:00" drop;ok meta time "2019-07-01 00:00:00" drop;ok meta time "2019-07-01 00:01:00" drop;ok diff --git a/tests/py/any/meta.t.payload b/tests/py/any/meta.t.payload index 402caae5cad8c..486d7aa566ea3 100644 --- a/tests/py/any/meta.t.payload +++ b/tests/py/any/meta.t.payload @@ -1050,7 +1050,7 @@ ip test-ip4 input # meta time "1970-05-23 21:07:14" drop ip meta-test input [ meta load time => reg 1 ] - [ cmp eq reg 1 0x74a8f400 0x002bd849 ] + [ cmp eq reg 1 0x43f05400 0x002bd503 ] [ immediate reg 0 drop ] # meta time 12341234 drop diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py index ce42b5ddb1cca..6edca3c6a5a2f 100755 --- a/tests/py/nft-test.py +++ b/tests/py/nft-test.py @@ -24,6 +24,7 @@ import tempfile TESTS_PATH = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/')) +os.environ['TZ'] = 'UTC-2' from nftables import Nftables
Payload generated for 'meta time' matches depends on host's timezone and DST setting. To produce constant output, set a fixed timezone in nft-test.py. Choose UTC-2 since most payloads are correct then, adjust the remaining two tests. Fixes: 0518ea3f70d8c ("tests: add meta time test cases") Signed-off-by: Phil Sutter <phil@nwl.cc> --- tests/py/any/meta.t | 2 +- tests/py/any/meta.t.payload | 2 +- tests/py/nft-test.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-)