[ovs-dev] [PATCH] ovn-northd: Handle IPv4 addresses with prefixes in lport port security

[Justin Pettit]

I think you might be able to write a slightly simpler patch by using ip_format_masked() like the following:

-=-=-=-=-=-=-=-=-=-
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4b1d611..890b17c 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1179,8 +1179,11 @@ build_port_security_nd(struct ovn_port *op, struct hmap *
             if (ps.n_ipv4_addrs) {
                 ds_put_cstr(&match, " && (");
                 for (size_t i = 0; i < ps.n_ipv4_addrs; i++) {
-                    ds_put_format(&match, "arp.spa == "IP_FMT" || ",
-                                  IP_ARGS(ps.ipv4_addrs[i].addr));
+                    ds_put_cstr(&match, "arp.spa == ");
+                    ip_format_masked(ps.ipv4_addrs[i].addr,
+                                     be32_prefix_mask(ps.ipv4_addrs[i].plen),
+                                     &match);
+                    ds_put_cstr(&match, " || ");
                 }
                 ds_chomp(&match, ' ');
                 ds_chomp(&match, '|');
@@ -1264,7 +1267,10 @@ build_port_security_ip(enum ovn_pipeline pipeline, struct
             }
 
             for (int i = 0; i < ps.n_ipv4_addrs; i++) {
-                ds_put_format(&match, IP_FMT", ", IP_ARGS(ps.ipv4_addrs[i].addr
+                ip_format_masked(ps.ipv4_addrs[i].addr,
+                                 be32_prefix_mask(ps.ipv4_addrs[i].plen),
+                                 &match);
+                ds_put_cstr(&match, ", ");
             }
 
             /* Replace ", " by "}". */
-=-=-=-=-=-=-=-=-=-

What do you think?

--Justin


> On Apr 6, 2016, at 8:18 AM, Numan Siddique <nusiddiq at redhat.com> wrote:
> 
> Initial implementation of port security, missed out this feature.
> 
> Reported-by: Na Zhu <nazhu at cn.ibm.com>
> Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1564414
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> ---
> ovn/northd/ovn-northd.c | 31 ++++++++++++++++++++++++++++---
> tests/ovn.at            | 19 +++++++++++++++++++
> 2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 4b1d611..975ca23 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1048,6 +1048,16 @@ extract_lport_addresses(char *address, struct lport_addresses *laddrs,
>     return true;
> }
> 
> +static inline bool
> +is_host_part_zero(ovs_be32 ip4, unsigned int plen)
> +{
> +    ovs_be32 mask = be32_prefix_mask(plen);
> +    if (plen != 32 && (ip4 & mask) == ip4) {
> +        return true;
> +    }
> +    return false;
> +}
> +
> /* Appends port security constraints on L2 address field 'eth_addr_field'
>  * (e.g. "eth.src" or "eth.dst") to 'match'.  'port_security', with
>  * 'n_port_security' elements, is the collection of port_security constraints
> @@ -1179,8 +1189,15 @@ build_port_security_nd(struct ovn_port *op, struct hmap *lflows)
>             if (ps.n_ipv4_addrs) {
>                 ds_put_cstr(&match, " && (");
>                 for (size_t i = 0; i < ps.n_ipv4_addrs; i++) {
> -                    ds_put_format(&match, "arp.spa == "IP_FMT" || ",
> -                                  IP_ARGS(ps.ipv4_addrs[i].addr));
> +                    if (is_host_part_zero(ps.ipv4_addrs[i].addr,
> +                                          ps.ipv4_addrs[i].plen)) {
> +                        ds_put_format(&match, "arp.spa == "IP_FMT"/%d || ",
> +                                      IP_ARGS(ps.ipv4_addrs[i].addr),
> +                                      ps.ipv4_addrs[i].plen);
> +                    } else {
> +                        ds_put_format(&match, "arp.spa == "IP_FMT" || ",
> +                                      IP_ARGS(ps.ipv4_addrs[i].addr));
> +                    }
>                 }
>                 ds_chomp(&match, ' ');
>                 ds_chomp(&match, '|');
> @@ -1264,7 +1281,15 @@ build_port_security_ip(enum ovn_pipeline pipeline, struct ovn_port *op,
>             }
> 
>             for (int i = 0; i < ps.n_ipv4_addrs; i++) {
> -                ds_put_format(&match, IP_FMT", ", IP_ARGS(ps.ipv4_addrs[i].addr));
> +                if (is_host_part_zero(ps.ipv4_addrs[i].addr,
> +                                      ps.ipv4_addrs[i].plen)) {
> +                    ds_put_format(&match, IP_FMT"/%d, ",
> +                                  IP_ARGS(ps.ipv4_addrs[i].addr),
> +                                  ps.ipv4_addrs[i].plen);
> +                } else {
> +                    ds_put_format(&match, IP_FMT", ",
> +                                  IP_ARGS(ps.ipv4_addrs[i].addr));
> +                }
>             }
> 
>             /* Replace ", " by "}". */
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 22121e1..441dd2b 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1930,6 +1930,25 @@ for i in 1 2 3; do
>     test_ipv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip $tip
> done
> 
> +# configure lport13 to send and received IPv4 packets with an address range
> +ovn-nbctl lport-set-port-security lp13 "f0:00:00:00:00:13 192.168.0.13 10.0.0.0/24"
> +
> +sip=`ip_to_hex 10 0 0 14`
> +tip=`ip_to_hex 192 168 0 23`
> +# IPv4 packet from lport13 with src ip 10.0.0.14 destined to lport23
> +# with dst ip 192.168.0.23 should be allowed
> +test_ip 13 f00000000013 f00000000023 $sip $tip 23
> +
> +sip=`ip_to_hex 192 168 0 33`
> +tip=`ip_to_hex 10 0 0 15`
> +# IPv4 packet from lport33 with src ip 192.168.0.33 destined to lport13
> +# with dst ip 10.0.0.15 should be received by lport13
> +test_ip 33 f00000000033 f00000000013 $sip $tip 13
> +
> +sip=`ip_to_hex 10 0 0 13`
> +tip=`ip_to_hex 192 168 0 22`
> +# arp packet with inner ip 10.0.0.13 should be allowed for lport13
> +test_arp 13 f00000000013 f00000000013 $sip $tip 0 f00000000022
> 
> # Allow some time for packet forwarding.
> 
> -- 
> 2.5.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev