[ovs-dev,RFC] OVN: add mac address only support to IPAM/MACAM
diff mbox series

Message ID 8efa4ffabbc2278dace5ca348852a6a374bc9445.1542369451.git.lorenzo.bianconi@redhat.com
State RFC
Headers show
Series
  • [ovs-dev,RFC] OVN: add mac address only support to IPAM/MACAM
Related show

Commit Message

Lorenzo Bianconi Nov. 16, 2018, noon UTC
Add the capability to assign just the L2 address to IPAM/MACAM since
in the current implementation either subnet or ipv6_prefix are mandatory
to enable IPAM

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 ovn/northd/ovn-northd.c |  8 +++++++-
 ovn/ovn-nb.xml          |  5 +++++
 tests/ovn.at            | 17 +++++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

Comments

Mark Michelson Nov. 16, 2018, 9:05 p.m. UTC | #1
Hi Lorenzo,

Since I'm aware of the request from within Red Hat for this 
functionality, I'm fine with this being added in.

I have some notes in-line below about the specific implementation.

On 11/16/2018 07:00 AM, Lorenzo Bianconi wrote:
> Add the capability to assign just the L2 address to IPAM/MACAM since
> in the current implementation either subnet or ipv6_prefix are mandatory
> to enable IPAM
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>   ovn/northd/ovn-northd.c |  8 +++++++-
>   ovn/ovn-nb.xml          |  5 +++++
>   tests/ovn.at            | 17 +++++++++++++++++
>   3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 4580cd705..9e547c7b2 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -414,6 +414,7 @@ struct ipam_info {
>       unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
>       bool ipv6_prefix_set;
>       struct in6_addr ipv6_prefix;
> +    bool mac_only;
>   };
>   
>   /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
> @@ -559,6 +560,10 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
>       }
>   
>       if (!subnet_str) {
> +        if (!ipv6_prefix) {
> +            od->ipam_info.mac_only = smap_get_bool(&od->nbs->other_config,
> +                                                   "mac_only", false);
> +        }
>           return;
>       }
>   
> @@ -1382,7 +1387,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
>               const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
>   
>               if (!od->ipam_info.allocated_ipv4s &&
> -                !od->ipam_info.ipv6_prefix_set) {
> +                !od->ipam_info.ipv6_prefix_set &&
> +                !od->ipam_info.mac_only) {
>                   if (nbsp->dynamic_addresses) {
>                       nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
>                                                                       NULL);
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 474b4f9a7..a5523ee18 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -267,6 +267,11 @@
>             <li><code>8230:5678::</code></li>
>           </ul>
>         </column>
> +
> +      <column name="other_config" key="mac_only">
> +        Boolean value used to request to assing L2 address only if neigter subnet
s/assing/assign/
s/neigter/neither/

> +        nor ipv6_prefix are specified
> +      </column>
>       </group>
>   
>       <group title="Common Columns">
> diff --git a/tests/ovn.at b/tests/ovn.at
> index ab32faa6b..15c96976a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5633,6 +5633,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 dynamic_addresses], [0],
>       ["00:11:22:a8:64:05 192.168.100.4"
>   ])
>   
> +# request to assign mac only
> +#
> +ovn-nbctl ls-add sw7
> +ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
> +for n in $(seq 1 3); do
> +    ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" dynamic
> +done
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
> +    ["00:11:22:00:00:03"
> +])
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
> +    ["00:11:22:00:00:04"
> +])
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
> +    ["00:11:22:00:00:05"
> +])
> +

Why do these MACs begin with 00:11:22? Since you didn't set a mac_prefix 
on sw7, shouldn't they start with 0a:00:00?

>   as ovn-sb
>   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>   
>
Lorenzo Bianconi Nov. 16, 2018, 9:51 p.m. UTC | #2
> Hi Lorenzo,
>
> Since I'm aware of the request from within Red Hat for this
> functionality, I'm fine with this being added in.
>
> I have some notes in-line below about the specific implementation.

Hi Mark,

thx for the review

>
> On 11/16/2018 07:00 AM, Lorenzo Bianconi wrote:
> > Add the capability to assign just the L2 address to IPAM/MACAM since
> > in the current implementation either subnet or ipv6_prefix are mandatory
> > to enable IPAM
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> > ---
> >   ovn/northd/ovn-northd.c |  8 +++++++-
> >   ovn/ovn-nb.xml          |  5 +++++
> >   tests/ovn.at            | 17 +++++++++++++++++
> >   3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 4580cd705..9e547c7b2 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -414,6 +414,7 @@ struct ipam_info {
> >       unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
> >       bool ipv6_prefix_set;
> >       struct in6_addr ipv6_prefix;
> > +    bool mac_only;
> >   };
> >
> >   /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
> > @@ -559,6 +560,10 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
> >       }
> >
> >       if (!subnet_str) {
> > +        if (!ipv6_prefix) {
> > +            od->ipam_info.mac_only = smap_get_bool(&od->nbs->other_config,
> > +                                                   "mac_only", false);
> > +        }
> >           return;
> >       }
> >
> > @@ -1382,7 +1387,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
> >               const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
> >
> >               if (!od->ipam_info.allocated_ipv4s &&
> > -                !od->ipam_info.ipv6_prefix_set) {
> > +                !od->ipam_info.ipv6_prefix_set &&
> > +                !od->ipam_info.mac_only) {
> >                   if (nbsp->dynamic_addresses) {
> >                       nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
> >                                                                       NULL);
> > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> > index 474b4f9a7..a5523ee18 100644
> > --- a/ovn/ovn-nb.xml
> > +++ b/ovn/ovn-nb.xml
> > @@ -267,6 +267,11 @@
> >             <li><code>8230:5678::</code></li>
> >           </ul>
> >         </column>
> > +
> > +      <column name="other_config" key="mac_only">
> > +        Boolean value used to request to assing L2 address only if neigter subnet
> s/assing/assign/
> s/neigter/neither/
>

ack

> > +        nor ipv6_prefix are specified
> > +      </column>
> >       </group>
> >
> >       <group title="Common Columns">
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index ab32faa6b..15c96976a 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -5633,6 +5633,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 dynamic_addresses], [0],
> >       ["00:11:22:a8:64:05 192.168.100.4"
> >   ])
> >
> > +# request to assign mac only
> > +#
> > +ovn-nbctl ls-add sw7
> > +ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
> > +for n in $(seq 1 3); do
> > +    ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" dynamic
> > +done
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
> > +    ["00:11:22:00:00:03"
> > +])
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
> > +    ["00:11:22:00:00:04"
> > +])
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
> > +    ["00:11:22:00:00:05"
> > +])
> > +
>
> Why do these MACs begin with 00:11:22? Since you didn't set a mac_prefix
> on sw7, shouldn't they start with 0a:00:00?
>

mac_prefix is global for deployment and it has been set in the previous test

Regards,
Lorenzo

> >   as ovn-sb
> >   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> >
> >
>

Patch
diff mbox series

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4580cd705..9e547c7b2 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -414,6 +414,7 @@  struct ipam_info {
     unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
     bool ipv6_prefix_set;
     struct in6_addr ipv6_prefix;
+    bool mac_only;
 };
 
 /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
@@ -559,6 +560,10 @@  init_ipam_info_for_datapath(struct ovn_datapath *od)
     }
 
     if (!subnet_str) {
+        if (!ipv6_prefix) {
+            od->ipam_info.mac_only = smap_get_bool(&od->nbs->other_config,
+                                                   "mac_only", false);
+        }
         return;
     }
 
@@ -1382,7 +1387,8 @@  build_ipam(struct hmap *datapaths, struct hmap *ports)
             const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
 
             if (!od->ipam_info.allocated_ipv4s &&
-                !od->ipam_info.ipv6_prefix_set) {
+                !od->ipam_info.ipv6_prefix_set &&
+                !od->ipam_info.mac_only) {
                 if (nbsp->dynamic_addresses) {
                     nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
                                                                     NULL);
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 474b4f9a7..a5523ee18 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -267,6 +267,11 @@ 
           <li><code>8230:5678::</code></li>
         </ul>
       </column>
+
+      <column name="other_config" key="mac_only">
+        Boolean value used to request to assing L2 address only if neigter subnet
+        nor ipv6_prefix are specified
+      </column>
     </group>
 
     <group title="Common Columns">
diff --git a/tests/ovn.at b/tests/ovn.at
index ab32faa6b..15c96976a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5633,6 +5633,23 @@  AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 dynamic_addresses], [0],
     ["00:11:22:a8:64:05 192.168.100.4"
 ])
 
+# request to assign mac only
+#
+ovn-nbctl ls-add sw7
+ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
+for n in $(seq 1 3); do
+    ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" dynamic
+done
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
+    ["00:11:22:00:00:03"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
+    ["00:11:22:00:00:04"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
+    ["00:11:22:00:00:05"
+])
+
 as ovn-sb
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])