On 6/23/20 11:08 AM, Dumitru Ceara wrote:
On 6/23/20 4:56 PM, Mark Michelson wrote:
Acked-by: Mark Michelson <[email protected]>
Thanks Mark for the review.
I was going to suggest printing an informational message if the value
gets truncated. However, given the way the lexer works, it's not as
simple as just printing a message when you encounter the situation. If
we were to add informational messages to the lexer, that should be a
separate (and very low-priority) patch.
I was looking into that too but couldn't find a not-so-intrusive and
clean way to do it. Do you have any suggestions? I can try to send a
follow up patch if we think it's worth it.
I gave it about 30 seconds of thought and couldn't think of anything
non-intrusive either. I also have to admit I don't have a ton of
experience in the lexer code, so it would take some research to figure
out a good way to change this.
I don't think it's worth following up immediately with a separate patch.
I think this gets filed into our "it would be nice to have" pile.
Thanks,
Dumitru
On 6/23/20 4:17 AM, Dumitru Ceara wrote:
It's quite restrictive to not accept ACLs/policies that match on a CIDR
that has non-zero host bits. Right now this generates a lexer error that
can only be detected in the logs.
There's no real harm in automatically zero-ing the unmasked bits.
Reported-at: https://bugzilla.redhat.com/1812820
Reported-by: Ying Xu <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
lib/lex.c | 10 ++--------
tests/ovn.at | 8 ++++----
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/lib/lex.c b/lib/lex.c
index 94f6c77..4d92199 100644
--- a/lib/lex.c
+++ b/lib/lex.c
@@ -485,16 +485,10 @@ lex_parse_mask(const char *p, struct lex_token
*token)
return p;
}
- /* Check invariant that a 1-bit in the value corresponds to a
1-bit in the
+ /* Apply invariant that a 1-bit in the value corresponds to a
1-bit in the
* mask. */
for (int i = 0; i < ARRAY_SIZE(token->mask.be32); i++) {
- ovs_be32 v = token->value.be32[i];
- ovs_be32 m = token->mask.be32[i];
-
- if (v & ~m) {
- lex_error(token, "Value contains unmasked 1-bits.");
- break;
- }
+ token->value.be32[i] &= token->mask.be32[i];
}
/* Done! */
diff --git a/tests/ovn.at b/tests/ovn.at
index 1ff7952..0c0daed 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -79,7 +79,7 @@ a/b => a error("`/' is only valid as part of `//' or
`/*'.") b
0/0
0/1
-1/0 => error("Value contains unmasked 1-bits.")
+1/0 => 0/0
1/1
128/384
1/3
@@ -99,7 +99,7 @@ a/b => a error("`/' is only valid as part of `//' or
`/*'.") b
0X => error("Hex digits expected following 0X.")
0x0/0x0 => 0/0
0x0/0x1 => 0/0x1
-0x1/0x0 => error("Value contains unmasked 1-bits.")
+0x1/0x0 => 0/0
0xffff/0x1ffff
0x. => error("Invalid syntax in hexadecimal constant.")
@@ -109,7 +109,7 @@ a/b => a error("`/' is only valid as part of
`//' or `/*'.") b
192.168.0.0/255.255.0.0 => 192.168.0.0/16
192.168.0.0/255.255.255.0 => 192.168.0.0/24
192.168.0.0/255.255.0.255
-192.168.0.0/255.0.0.0 => error("Value contains unmasked 1-bits.")
+192.168.0.0/255.0.0.0 => 192.0.0.0/8
192.168.0.0/32
192.168.0.0/255.255.255.255 => 192.168.0.0/32
1.2.3.4:5 => 1.2.3.4 : 5
@@ -135,7 +135,7 @@ FE:DC:ba:98:76:54 => fe:dc:ba:98:76:54
01:00:00:00:00:00/01:00:00:00:00:00
ff:ff:ff:ff:ff:ff/ff:ff:ff:ff:ff:ff
fe:ff:ff:ff:ff:ff/ff:ff:ff:ff:ff:ff
-ff:ff:ff:ff:ff:ff/fe:ff:ff:ff:ff:ff => error("Value contains unmasked
1-bits.")
+ff:ff:ff:ff:ff:ff/fe:ff:ff:ff:ff:ff =>
fe:ff:ff:ff:ff:ff/fe:ff:ff:ff:ff:ff
fe:x => error("Invalid numeric constant.")
00:01:02:03:04:x => error("Invalid numeric constant.")
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
