Acked-by: Mark Michelson <mmichels at redhat.com>
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.
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 <yinxu at redhat.com>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
> 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.")
>
>