diff mbox series

[ovs-dev,ovn] lex: Allow unmasked bits in value/mask tokens.

Message ID 1592900270-30788-1-git-send-email-dceara@redhat.com
State Accepted
Headers show
Series [ovs-dev,ovn] lex: Allow unmasked bits in value/mask tokens. | expand

Commit Message

Dumitru Ceara June 23, 2020, 8:17 a.m. UTC
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@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 lib/lex.c    | 10 ++--------
 tests/ovn.at |  8 ++++----
 2 files changed, 6 insertions(+), 12 deletions(-)

Comments

Mark Michelson June 23, 2020, 2:56 p.m. UTC | #1
Acked-by: Mark Michelson <mmichels@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@redhat.com>
> Signed-off-by: Dumitru Ceara <dceara@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.")
>   
>
Dumitru Ceara June 23, 2020, 3:08 p.m. UTC | #2
On 6/23/20 4:56 PM, Mark Michelson wrote:
> Acked-by: Mark Michelson <mmichels@redhat.com>
> 

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 <yinxu@redhat.com>
>> Signed-off-by: Dumitru Ceara <dceara@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.")
>>  
>
Mark Michelson June 23, 2020, 4:33 p.m. UTC | #3
On 6/23/20 11:08 AM, Dumitru Ceara wrote:
> On 6/23/20 4:56 PM, Mark Michelson wrote:
>> Acked-by: Mark Michelson <mmichels@redhat.com>
>>
> 
> 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 <yinxu@redhat.com>
>>> Signed-off-by: Dumitru Ceara <dceara@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.")
>>>   
>>
>
Numan Siddique June 24, 2020, 10:47 a.m. UTC | #4
On Tue, Jun 23, 2020 at 10:04 PM Mark Michelson <mmichels@redhat.com> wrote:

> On 6/23/20 11:08 AM, Dumitru Ceara wrote:
> > On 6/23/20 4:56 PM, Mark Michelson wrote:
> >> Acked-by: Mark Michelson <mmichels@redhat.com>
>

Thanks Dumitru and Mark (for the reviews).

I applied this patch to master with the below changes in the tests/ovn.at.
I was just playing around
how the patch works and thought we could add this to the test.

diff --git a/tests/ovn.at b/tests/ovn.at
index de62f3883..6587fd8ff 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -112,6 +112,9 @@ a/b => a error("`/' is only valid as part of `//' or
`/*'.") b
 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
+192.168.0.2/32
+192.168.0.2/30 => 192.168.0.0/30
+192.168.0.2/24 => 192.168.0.0/24
 1.2.3.4:5 => 1.2.3.4 : 5

 ::

Thanks
Numan

>>
> >
> > 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 <yinxu@redhat.com>
> >>> Signed-off-by: Dumitru Ceara <dceara@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.")
> >>>
> >>
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Dumitru Ceara June 24, 2020, 11:04 a.m. UTC | #5
On 6/24/20 12:47 PM, Numan Siddique wrote:
> 
> 
> On Tue, Jun 23, 2020 at 10:04 PM Mark Michelson <mmichels@redhat.com
> <mailto:mmichels@redhat.com>> wrote:
> 
>     On 6/23/20 11:08 AM, Dumitru Ceara wrote:
>     > On 6/23/20 4:56 PM, Mark Michelson wrote:
>     >> Acked-by: Mark Michelson <mmichels@redhat.com
>     <mailto:mmichels@redhat.com>>
> 
> 
> Thanks Dumitru and Mark (for the reviews).
> 
> I applied this patch to master with the below changes in the
> tests/ovn.at <http://ovn.at>. I was just playing around
> how the patch works and thought we could add this to the test.
> 
> diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at>
> index de62f3883..6587fd8ff 100644
> --- a/tests/ovn.at <http://ovn.at>
> +++ b/tests/ovn.at <http://ovn.at>
> @@ -112,6 +112,9 @@ a/b => a error("`/' is only valid as part of `//' or
> `/*'.") b
>  192.168.0.0/255.0.0.0 <http://192.168.0.0/255.0.0.0> => 192.0.0.0/8
> <http://192.0.0.0/8>
>  192.168.0.0/32 <http://192.168.0.0/32>
>  192.168.0.0/255.255.255.255 <http://192.168.0.0/255.255.255.255> =>
> 192.168.0.0/32 <http://192.168.0.0/32>
> +192.168.0.2/32 <http://192.168.0.2/32>
> +192.168.0.2/30 <http://192.168.0.2/30> => 192.168.0.0/30
> <http://192.168.0.0/30>
> +192.168.0.2/24 <http://192.168.0.2/24> => 192.168.0.0/24
> <http://192.168.0.0/24>
>  1.2.3.4:5 <http://1.2.3.4:5> => 1.2.3.4 : 5
>  
>  ::
> 
> Thanks
> Numan
> 

Looks good to me, thanks!

Regards,
Dumitru
diff mbox series

Patch

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