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. 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
