| Message ID | 1497641742-25272-1-git-send-email-mayhs11saini@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: > This test checks bug identified and fixed in the commit mentioned below > In a statement if there are multiple src data then it would be > totally ambiguous to decide which value to set. > > Before the commit was made it returned 134(BUG), but now it returns 1 > i.e, an error message. Applied, thanks. One change though before applying, see below. > Test: 986dea8 ("evaluate: avoid reference to multiple src data in > statements which set values") > Signed-off-by: Shyam Saini <mayhs11saini@gmail.com> > --- > tests/py/any/ct.t | 10 ++++++++++ > tests/py/any/meta.t | 8 ++++++++ > tests/py/bridge/ether.t | 7 +++++++ > tests/py/inet/tcp.t | 7 +++++++ > tests/py/inet/udp.t | 7 +++++++ > tests/py/ip/ip.t | 7 +++++++ > 6 files changed, 46 insertions(+) > > diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t > index 20f047a..f7c2321 100644 > --- a/tests/py/any/ct.t > +++ b/tests/py/any/ct.t > @@ -58,6 +58,16 @@ ct mark set 0x11;ok;ct mark set 0x00000011 > ct mark set mark;ok;ct mark set mark > ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 };ok;ct mark set mark map { 0x00000003 : 0x0000001e, 0x00000002 : 0x00000014, 0x00000001 : 0x0000000a} > > +# Following rules tests ambguity while setting the value > +# sudo nft add rule ip test-ip4 output ct mark set {0x11333, 0x11} > +# <cmdline>:1:41-55: Error: you cannot use a set here, unknown value to use > +# add rule ip test-ip4 output ct mark set {0x11333, 0x11} > +# ~~~~~~~~~~~~^^^^^^^^^^^^^^^ No need to copy and paste this over and over again. Look, next time you place this in the commit message, we can find the reason why this line was added via: $ git annotate So I have removed them all before applying. -- 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 Sun, Jun 18, 2017 at 11:29:13AM +0200, Pablo Neira Ayuso wrote: > On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: > > This test checks bug identified and fixed in the commit mentioned below > > In a statement if there are multiple src data then it would be > > totally ambiguous to decide which value to set. > > > > Before the commit was made it returned 134(BUG), but now it returns 1 > > i.e, an error message. > > Applied, thanks. > > One change though before applying, see below. > > > Test: 986dea8 ("evaluate: avoid reference to multiple src data in > > statements which set values") BTW, please update .gitconfig in your home to add: [core] abbrev = 12 So we get commit abbrev of 12 chars at least. 7 may lead to ambiguities in the future, as a hash with the same prefix may show up later on. 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
On Sun, Jun 18, 2017 at 11:29:13AM +0200, Pablo Neira Ayuso wrote: > On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: > > This test checks bug identified and fixed in the commit mentioned below > > In a statement if there are multiple src data then it would be > > totally ambiguous to decide which value to set. > > > > Before the commit was made it returned 134(BUG), but now it returns 1 > > i.e, an error message. > > Applied, thanks. > > One change though before applying, see below. BTW, shouldn't we check for explicit exit code 1 in rule_add() in tests/py/? > > Test: 986dea8 ("evaluate: avoid reference to multiple src data in > > statements which set values") It would be good to run this test with and without 986dea8. If we hit exit code 134, the py test should complain even if we say "fail". Basically, test py with 'fail' is fine if we fail gracefully, not if we hit BUG. Probably just a matter of making a oneline patch for nft-tests.py to enforce this? -- 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 Sun, Jun 18, 2017 at 2:59 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: >> This test checks bug identified and fixed in the commit mentioned below >> In a statement if there are multiple src data then it would be >> totally ambiguous to decide which value to set. >> >> Before the commit was made it returned 134(BUG), but now it returns 1 >> i.e, an error message. > > Applied, thanks. > > One change though before applying, see below. > >> Test: 986dea8 ("evaluate: avoid reference to multiple src data in >> statements which set values") >> Signed-off-by: Shyam Saini <mayhs11saini@gmail.com> >> --- >> tests/py/any/ct.t | 10 ++++++++++ >> tests/py/any/meta.t | 8 ++++++++ >> tests/py/bridge/ether.t | 7 +++++++ >> tests/py/inet/tcp.t | 7 +++++++ >> tests/py/inet/udp.t | 7 +++++++ >> tests/py/ip/ip.t | 7 +++++++ >> 6 files changed, 46 insertions(+) >> >> diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t >> index 20f047a..f7c2321 100644 >> --- a/tests/py/any/ct.t >> +++ b/tests/py/any/ct.t >> @@ -58,6 +58,16 @@ ct mark set 0x11;ok;ct mark set 0x00000011 >> ct mark set mark;ok;ct mark set mark >> ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 };ok;ct mark set mark map { 0x00000003 : 0x0000001e, 0x00000002 : 0x00000014, 0x00000001 : 0x0000000a} >> >> +# Following rules tests ambguity while setting the value >> +# sudo nft add rule ip test-ip4 output ct mark set {0x11333, 0x11} >> +# <cmdline>:1:41-55: Error: you cannot use a set here, unknown value to use >> +# add rule ip test-ip4 output ct mark set {0x11333, 0x11} >> +# ~~~~~~~~~~~~^^^^^^^^^^^^^^^ > > No need to copy and paste this over and over again. > > Look, next time you place this in the commit message, we can find the > reason why this line was added via: Sure Thanks > $ git annotate > > So I have removed them all before applying. -- 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 Sun, Jun 18, 2017 at 3:01 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sun, Jun 18, 2017 at 11:29:13AM +0200, Pablo Neira Ayuso wrote: >> On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: >> > This test checks bug identified and fixed in the commit mentioned below >> > In a statement if there are multiple src data then it would be >> > totally ambiguous to decide which value to set. >> > >> > Before the commit was made it returned 134(BUG), but now it returns 1 >> > i.e, an error message. >> >> Applied, thanks. >> >> One change though before applying, see below. >> >> > Test: 986dea8 ("evaluate: avoid reference to multiple src data in >> > statements which set values") > > BTW, please update .gitconfig in your home to add: > > [core] > abbrev = 12 > > So we get commit abbrev of 12 chars at least. > > 7 may lead to ambiguities in the future, as a hash with the same > prefix may show up later on. Done Thanks a lot :) > 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
On Sun, Jun 18, 2017 at 3:18 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sun, Jun 18, 2017 at 11:29:13AM +0200, Pablo Neira Ayuso wrote: >> On Sat, Jun 17, 2017 at 01:05:42AM +0530, Shyam Saini wrote: >> > This test checks bug identified and fixed in the commit mentioned below >> > In a statement if there are multiple src data then it would be >> > totally ambiguous to decide which value to set. >> > >> > Before the commit was made it returned 134(BUG), but now it returns 1 >> > i.e, an error message. >> >> Applied, thanks. >> >> One change though before applying, see below. > > BTW, shouldn't we check for explicit exit code 1 in rule_add() in > tests/py/? > >> > Test: 986dea8 ("evaluate: avoid reference to multiple src data in >> > statements which set values") > > It would be good to run this test with and without 986dea8. > > If we hit exit code 134, the py test should complain even if we say > "fail". > > Basically, test py with 'fail' is fine if we fail gracefully, not if > we hit BUG. > > Probably just a matter of making a oneline patch for nft-tests.py to > enforce this? Patch sent, 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/tests/py/any/ct.t b/tests/py/any/ct.t index 20f047a..f7c2321 100644 --- a/tests/py/any/ct.t +++ b/tests/py/any/ct.t @@ -58,6 +58,16 @@ ct mark set 0x11;ok;ct mark set 0x00000011 ct mark set mark;ok;ct mark set mark ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 };ok;ct mark set mark map { 0x00000003 : 0x0000001e, 0x00000002 : 0x00000014, 0x00000001 : 0x0000000a} +# Following rules tests ambguity while setting the value +# sudo nft add rule ip test-ip4 output ct mark set {0x11333, 0x11} +# <cmdline>:1:41-55: Error: you cannot use a set here, unknown value to use +# add rule ip test-ip4 output ct mark set {0x11333, 0x11} +# ~~~~~~~~~~~~^^^^^^^^^^^^^^^ +ct mark set {0x11333, 0x11};fail +ct zone set {123, 127};fail +ct label set {123, 127};fail +ct event set {new, related, destroy, label};fail + ct expiration 30;ok;ct expiration 30s ct expiration 22;ok;ct expiration 22s ct expiration != 233;ok;ct expiration != 3m53s diff --git a/tests/py/any/meta.t b/tests/py/any/meta.t index 2ff942f..e19a8f9 100644 --- a/tests/py/any/meta.t +++ b/tests/py/any/meta.t @@ -143,6 +143,14 @@ meta mark set 0xffffffde or 0x16;ok;mark set 0xffffffde meta mark set 0x32 or 0xfffff;ok;mark set 0x000fffff meta mark set 0xfffe xor 0x16;ok;mark set 0x0000ffe8 +# Test ambiguity while setting the value +# sudo nft add rule ip test-ip4 input meta mark set {0xffff, 0xcc} +# <cmdline>:1:42-55: Error: you cannot use a set here, unknown value to use +# add rule ip test-ip4 input meta mark set {0xffff, 0xcc} +# ~~~~~~~~~~~~~~^^^^^^^^^^^^^^ +meta mark set {0xffff, 0xcc};fail +meta pkttype set {unicast, multicast, broadcast};fail + meta iif "lo";ok;iif "lo" meta oif "lo";ok;oif "lo" meta oifname "dummy2" accept;ok;oifname "dummy2" accept diff --git a/tests/py/bridge/ether.t b/tests/py/bridge/ether.t index 5c6766e..94637a6 100644 --- a/tests/py/bridge/ether.t +++ b/tests/py/bridge/ether.t @@ -8,3 +8,10 @@ tcp dport 22 ether saddr 00:0f:54:0c:11:04 ip daddr 1.2.3.4;ok ether saddr 00:0f:54:0c:11:04 ip daddr 1.2.3.4 accept;ok ether daddr 00:01:02:03:04:05 ether saddr set ff:fe:dc:ba:98:76 drop;ok + +#Test ambiguity while setting the value +#sudo nft add rule bridge test-bridge input ether daddr set {01:00:5e:00:01:01, 01:00:5e:00:02:02} +#<cmdline>:1:51-88: Error: you cannot use a set here, unknown value to use +#add rule bridge test-bridge input ether daddr set {01:00:5e:00:01:01, 01:00:5e:00:02:02} +# ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +ether daddr set {01:00:5e:00:01:01, 01:00:5e:00:02:02};fail diff --git a/tests/py/inet/tcp.t b/tests/py/inet/tcp.t index 5a0aab6..0281387 100644 --- a/tests/py/inet/tcp.t +++ b/tests/py/inet/tcp.t @@ -6,6 +6,13 @@ *inet;test-inet;input *netdev;test-netdev;ingress +# Test ambiguity while setting the value +# sudo nft add rule ip test-ip4 input tcp dport set {1, 2, 3} +# <cmdline>:1:42-50: Error: you cannot use a set here, unknown value to use +# add rule ip test-ip4 input tcp dport set {1, 2, 3} +# ~~~~~~~~~~~~~~^^^^^^^^^ +tcp dport set {1, 2, 3};fail + tcp dport 22;ok tcp dport != 233;ok tcp dport 33-45;ok diff --git a/tests/py/inet/udp.t b/tests/py/inet/udp.t index 2f16e6a..bb09973 100644 --- a/tests/py/inet/udp.t +++ b/tests/py/inet/udp.t @@ -15,6 +15,13 @@ udp sport != { 50, 60} accept;ok udp sport { 12-40};ok udp sport != { 13-24};ok +# Test ambiguity while the value +# sudo nft add rule ip test-ip4 input udp dport set {1, 2, 3} +# <cmdline>:1:42-50: Error: you cannot use a set here, unknown value to use +# add rule ip test-ip4 input udp dport set {1, 2, 3} +# ~~~~~~~~~~~~~~^^^^^^^^^ +udp dport set {1, 2, 3};fail + udp dport 80 accept;ok udp dport != 60 accept;ok udp dport 70-75 accept;ok diff --git a/tests/py/ip/ip.t b/tests/py/ip/ip.t index f984646..3fa511a 100644 --- a/tests/py/ip/ip.t +++ b/tests/py/ip/ip.t @@ -85,6 +85,13 @@ ip checksum != { 33, 55, 67, 88};ok ip checksum { 33-55};ok ip checksum != { 33-55};ok +# Test ambiguity while setting value +# sudo nft add rule ip test-ip4 input ip saddr set {192.19.1.2, 191.1.22.1} +# <cmdline>:1:41-64: Error: you cannot use a set here, unknown value to use +# add rule ip test-ip4 input ip saddr set {192.19.1.2, 191.1.22.1} +# ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^ +ip saddr set {192.19.1.2, 191.1.22.1};fail + ip saddr 192.168.2.0/24;ok ip saddr != 192.168.2.0/24;ok ip saddr 192.168.3.1 ip daddr 192.168.3.100;ok
This test checks bug identified and fixed in the commit mentioned below In a statement if there are multiple src data then it would be totally ambiguous to decide which value to set. Before the commit was made it returned 134(BUG), but now it returns 1 i.e, an error message. Test: 986dea8 ("evaluate: avoid reference to multiple src data in statements which set values") Signed-off-by: Shyam Saini <mayhs11saini@gmail.com> --- tests/py/any/ct.t | 10 ++++++++++ tests/py/any/meta.t | 8 ++++++++ tests/py/bridge/ether.t | 7 +++++++ tests/py/inet/tcp.t | 7 +++++++ tests/py/inet/udp.t | 7 +++++++ tests/py/ip/ip.t | 7 +++++++ 6 files changed, 46 insertions(+)