Re: [ovs-dev] [PATCH RFC v3] ovn-northd: add default_dhcpvx_options for Logical_Switch

[Zong Kai Li]

On Thu, Sep 15, 2016 at 7:48 AM, Ben Pfaff  wrote:
> On Thu, Sep 01, 2016 at 09:56:30AM +, Zongkai LI wrote:
>> This patch adds default_dhcpv4_options and default_dhcpv6_options columns for
>> Logical_Switch, which should help CMS not to calculate and set dhcpv4_options
>> and dhcpv6_options columns for lswitch ports on lswitchs one by one, when
>> most of lswitch ports on the same lswitch are using the DHCPv4_Options and
>> DHCPv6_Options. Default DHCP(v4 and v6) Options should benefit in case
>> scalling up and DB synchronization between CMS and OVN NB.
>>
>> v1 -> v2
>> add ACL lflows support for lswitch ports using default_dhcpvx_options from
>> lswitch.
>>
>> v2 -> v3
>> update ovn dhcpv4 and dhcpv6 tests for lswitch ports using specific dhcp
>> options than defualt ones in lswitch.
>
> The CMS can point the multiple dhcpvx_options column in as many rows as
> it likes to the same row in the DHCP_Options table, so I don't know why
> this offers an advantage.

Sorry for my poor english, I couldn't get your points well.
"The CMS can point the multiple dhcpvx_options column in as many rows
as it likes to the same row in the DHCP_Options table", do you mean
the CMS can point multiple options in a DHCP_Options row option column
? This patch won't help CMS on that.

The deep reason I submitted this patch is, we locate a DHCP_Options
row for Logical_Switch_Port.dhcpvx_options to refer by scanning
DHCP_Options table to find a row matching some rules we defined. Like
"subnet_id" and "port_id" defined in external_ids column, the
OpenStack integrator, networking-ovn uses them to locate a certain
DHCP_Options row for a port to set its dhcpvx_options.

It works, but not necessary for case [1] most ports from the same
logical-switch using the same dhcpvx_options, since we will locate the
same uuid duplicately.
It's possible for CMS to use some cache to store mappings between
networks/subnets and  DHCP_Options rows uuids, but I don't think this
is a CMS-friendly approach.

By adding default dhcpvx options in Logical_Switch, CMS will skip the
scanning step for ports using the default dhcp options cases, such as
creating a new port, logical switch port lost with/without dhcp
options lost(or become stale) when DB sync, and logical switch port
exist but dhcp options lost when DB sync.

This is a option way let CMS to handle dhcp options, it won't change
current approach how DHCP_Options and
Logical_Switch_Port.dhcpvx_options works. It won't benefit too much
scenarios, but at least for case [1] it would.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC v3] ovn-northd: add default_dhcpvx_options for Logical_Switch

[Ben Pfaff]

On Thu, Sep 01, 2016 at 09:56:30AM +, Zongkai LI wrote:
> This patch adds default_dhcpv4_options and default_dhcpv6_options columns for
> Logical_Switch, which should help CMS not to calculate and set dhcpv4_options
> and dhcpv6_options columns for lswitch ports on lswitchs one by one, when
> most of lswitch ports on the same lswitch are using the DHCPv4_Options and
> DHCPv6_Options. Default DHCP(v4 and v6) Options should benefit in case
> scalling up and DB synchronization between CMS and OVN NB.
> 
> v1 -> v2
> add ACL lflows support for lswitch ports using default_dhcpvx_options from
> lswitch.
> 
> v2 -> v3
> update ovn dhcpv4 and dhcpv6 tests for lswitch ports using specific dhcp
> options than defualt ones in lswitch.

The CMS can point the multiple dhcpvx_options column in as many rows as
it likes to the same row in the DHCP_Options table, so I don't know why
this offers an advantage.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH RFC v3] ovn-northd: add default_dhcpvx_options for Logical_Switch

[Zongkai LI]

This patch adds default_dhcpv4_options and default_dhcpv6_options columns for
Logical_Switch, which should help CMS not to calculate and set dhcpv4_options
and dhcpv6_options columns for lswitch ports on lswitchs one by one, when
most of lswitch ports on the same lswitch are using the DHCPv4_Options and
DHCPv6_Options. Default DHCP(v4 and v6) Options should benefit in case
scalling up and DB synchronization between CMS and OVN NB.

v1 -> v2
add ACL lflows support for lswitch ports using default_dhcpvx_options from
lswitch.

v2 -> v3
update ovn dhcpv4 and dhcpv6 tests for lswitch ports using specific dhcp
options than defualt ones in lswitch.
---
 ovn/northd/ovn-northd.c | 144 
 ovn/ovn-nb.ovsschema|  18 +-
 ovn/ovn-nb.xml  |  26 +
 tests/ovn.at|  74 +++--
 4 files changed, 220 insertions(+), 42 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 0ad9190..3ab11fa 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1784,14 +1784,27 @@ static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
 struct ds *options_action, struct ds *response_action)
 {
+char *cidr = NULL;
+struct smap dhcpv4_options = SMAP_INITIALIZER(_options);
 if (!op->nbsp->dhcpv4_options) {
-/* CMS has disabled native DHCPv4 for this lport. */
-return false;
+if ((!op->nbsp->use_default_dhcpv4 || *op->nbsp->use_default_dhcpv4)
+&& op->od->nbs->default_dhcpv4_options) {
+/* Use lswitch default DHCPv4 options for this lport. */
+cidr = op->od->nbs->default_dhcpv4_options->cidr;
+smap_clone(_options,
+   >od->nbs->default_dhcpv4_options->options);
+} else {
+/* CMS has disabled native DHCPv4 for this lport. */
+smap_destroy(_options);
+return false;
+}
+} else {
+cidr = op->nbsp->dhcpv4_options->cidr;
+smap_clone(_options, >nbsp->dhcpv4_options->options);
 }
 
 ovs_be32 host_ip, mask;
-char *error = ip_parse_masked(op->nbsp->dhcpv4_options->cidr, _ip,
-  );
+char *error = ip_parse_masked(cidr, _ip, );
 if (error || ((offer_ip ^ host_ip) & mask)) {
/* Either
 *  - cidr defined is invalid or
@@ -1802,14 +1815,10 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 return false;
 }
 
-const char *server_ip = smap_get(
->nbsp->dhcpv4_options->options, "server_id");
-const char *server_mac = smap_get(
->nbsp->dhcpv4_options->options, "server_mac");
-const char *lease_time = smap_get(
->nbsp->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
->nbsp->dhcpv4_options->options, "router");
+const char *server_ip = smap_get(_options, "server_id");
+const char *server_mac = smap_get(_options, "server_mac");
+const char *lease_time = smap_get(_options, "lease_time");
+const char *router = smap_get(_options, "router");
 
 if (!(server_ip && server_mac && lease_time && router)) {
 /* "server_id", "server_mac", "lease_time" and "router" should be
@@ -1820,8 +1829,8 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 return false;
 }
 
-struct smap dhcpv4_options = SMAP_INITIALIZER(_options);
-smap_clone(_options, >nbsp->dhcpv4_options->options);
+char server_mac_str[ETH_ADDR_STRLEN + 1];
+strcpy(server_mac_str, server_mac);
 
 /* server_mac is not DHCPv4 option, delete it from the smap. */
 smap_remove(_options, "server_mac");
@@ -1845,7 +1854,7 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
   "ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; "
   "udp.dst = 68; outport = inport; flags.loopback = 1; "
   "output;",
-  server_mac, IP_ARGS(offer_ip), server_ip);
+  server_mac_str, IP_ARGS(offer_ip), server_ip);
 
 smap_destroy(_options);
 return true;
@@ -1855,15 +1864,28 @@ static bool
 build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
 struct ds *options_action, struct ds *response_action)
 {
+char *cidr = NULL;
+struct smap dhcpv6_options = SMAP_INITIALIZER(_options);
 if (!op->nbsp->dhcpv6_options) {
-/* CMS has disabled native DHCPv6 for this lport. */
-return false;
+if ((!op->nbsp->use_default_dhcpv6 || *op->nbsp->use_default_dhcpv6)
+&& op->od->nbs->default_dhcpv6_options) {
+/* Use lswitch default DHCPv6 options for this lport. */
+cidr = op->od->nbs->default_dhcpv6_options->cidr;
+smap_clone(_options,
+   >od->nbs->default_dhcpv6_options->options);
+} else {
+/* CMS has disabled native DHCPv6