Re: [ovs-dev] [PATCH v3 1/2 ovn] Fix the data type for DHCP option tftp_server (66)
Hi Numan,
Thanks for reply.
Sure, added Signed-off-by tag.
Regards,
Ankur
From: Numan Siddique
Sent: Monday, June 1, 2020 10:10 AM
To: svc.mail.git
Cc: ovs-dev ; Dhathri Purohith
Subject: Re: [ovs-dev] [PATCH v3 1/2 ovn] Fix the data type for DHCP option
tftp_server (66)
On Mon, Jun 1, 2020 at 10:38 PM Ankur Sharma
mailto:svc.mail@nutanix.com>> wrote:
From: Dhathri Purohith
mailto:dhathri.puroh...@nutanix.com>>
Currently, DHCP option is of type ipv4. But, according to RFC 2132,
option 66 can be a hostname i.e, we should also accept string type.
In order to be backward compatible, a new type called "host_id" is
introduced, which accepts both ipv4 address and string. Type for DHCP
option 66 is changed to "host_id" instead of ipv4.
OVN northd code that updates the OVN southbound database is enhanced to
consider the change in the type and code for DHCP option, so that the
change in datatype is reflected.
Signed-off-by: Dhathri Purohith
mailto:dhathri.puroh...@nutanix.com>>
Hi Ankur,
Since you're submitting the patches, can you please provide your Signed-off-by
tag.
Thanks
Numan
---
lib/actions.c | 12
lib/ovn-l7.h| 2 +-
northd/ovn-northd.c | 7 ++-
ovn-sb.ovsschema| 7 ---
ovn-sb.xml | 13 +
tests/ovn.at
[ovn.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn.at=DwMFaQ=s883GpUCOChKOHiocYtGcg=gdsHrMKI-VOHoN1ziUyqlIGzgVzIjMMz-NHIsocZYPE=3SIs7Pp0sjH9HfYKcpws-SmpjWk4y-jhVHaxzGc_UxY=_TgI7ILfYmFnim1OhV9TsI-nUXmxlO8lgcefxgjWLzE=>
| 6 ++
tests/test-ovn.c| 2 +-
7 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/lib/actions.c b/lib/actions.c
index c506151..f21be6d 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1978,6 +1978,10 @@ parse_gen_opt(struct action_context *ctx, struct
ovnact_gen_option *o,
return;
}
+if (!strcmp(o->option->type, "host_id")) {
+return;
+}
+
if (!strcmp(o->option->type, "str")) {
if (o->value.type != EXPR_C_STRING) {
lexer_error(ctx->lexer, "%s option %s requires string value.",
@@ -2305,6 +2309,14 @@ encode_put_dhcpv4_option(const struct ovnact_gen_option
*o,
} else if (!strcmp(o->option->type, "str")) {
opt_header[1] = strlen(c->string);
ofpbuf_put(ofpacts, c->string, opt_header[1]);
+} else if (!strcmp(o->option->type, "host_id")) {
+if (o->value.type == EXPR_C_STRING) {
+opt_header[1] = strlen(c->string);
+ofpbuf_put(ofpacts, c->string, opt_header[1]);
+} else {
+ opt_header[1] = sizeof(ovs_be32);
+ ofpbuf_put(ofpacts, >value.ipv4, sizeof(ovs_be32));
+}
}
}
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index cc63d82..38555db 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -57,7 +57,7 @@ struct gen_opts_map {
#define DHCP_OPT_NIS_SERVER DHCP_OPTION("nis_server", 41, "ipv4")
#define DHCP_OPT_NTP_SERVER DHCP_OPTION("ntp_server", 42, "ipv4")
#define DHCP_OPT_SERVER_ID DHCP_OPTION("server_id", 54, "ipv4")
-#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "ipv4")
+#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "host_id")
#define DHCP_OPT_CLASSLESS_STATIC_ROUTE \
DHCP_OPTION("classless_static_route", 121, "static_routes")
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index eb78f31..ef08414 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11360,7 +11360,12 @@ check_and_add_supported_dhcp_opts_to_sb_db(struct
northd_context *ctx)
struct gen_opts_map *dhcp_opt =
dhcp_opts_find(_opts_to_add, opt_row->name);
if (dhcp_opt) {
-hmap_remove(_opts_to_add, _opt->hmap_node);
+if (!strcmp(dhcp_opt->type, opt_row->type) &&
+ dhcp_opt->code == opt_row->code) {
+hmap_remove(_opts_to_add, _opt->hmap_node);
+} else {
+sbrec_dhcp_options_delete(opt_row);
+}
} else {
sbrec_dhcp_options_delete(opt_row);
}
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index c196dda..2ec729b 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
-"version": "2.8.0",
-"cksum": "1643994484 21853",
+"version": "2.8.1",
+"cksum": "236203406 21905",
"tables": {
"SB_Global": {
"columns": {
@@ -217,7 +217,8 @@
"type": {"key": {
&quo
Re: [ovs-dev] [PATCH v3 1/2 ovn] Fix the data type for DHCP option tftp_server (66)
On Mon, Jun 1, 2020 at 10:38 PM Ankur Sharma
wrote:
> From: Dhathri Purohith
>
> Currently, DHCP option is of type ipv4. But, according to RFC 2132,
> option 66 can be a hostname i.e, we should also accept string type.
> In order to be backward compatible, a new type called "host_id" is
> introduced, which accepts both ipv4 address and string. Type for DHCP
> option 66 is changed to "host_id" instead of ipv4.
> OVN northd code that updates the OVN southbound database is enhanced to
> consider the change in the type and code for DHCP option, so that the
> change in datatype is reflected.
>
> Signed-off-by: Dhathri Purohith
>
Hi Ankur,
Since you're submitting the patches, can you please provide your
Signed-off-by tag.
Thanks
Numan
> ---
> lib/actions.c | 12
> lib/ovn-l7.h| 2 +-
> northd/ovn-northd.c | 7 ++-
> ovn-sb.ovsschema| 7 ---
> ovn-sb.xml | 13 +
> tests/ovn.at| 6 ++
> tests/test-ovn.c| 2 +-
> 7 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/lib/actions.c b/lib/actions.c
> index c506151..f21be6d 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1978,6 +1978,10 @@ parse_gen_opt(struct action_context *ctx, struct
> ovnact_gen_option *o,
> return;
> }
>
> +if (!strcmp(o->option->type, "host_id")) {
> +return;
> +}
> +
> if (!strcmp(o->option->type, "str")) {
> if (o->value.type != EXPR_C_STRING) {
> lexer_error(ctx->lexer, "%s option %s requires string value.",
> @@ -2305,6 +2309,14 @@ encode_put_dhcpv4_option(const struct
> ovnact_gen_option *o,
> } else if (!strcmp(o->option->type, "str")) {
> opt_header[1] = strlen(c->string);
> ofpbuf_put(ofpacts, c->string, opt_header[1]);
> +} else if (!strcmp(o->option->type, "host_id")) {
> +if (o->value.type == EXPR_C_STRING) {
> +opt_header[1] = strlen(c->string);
> +ofpbuf_put(ofpacts, c->string, opt_header[1]);
> +} else {
> + opt_header[1] = sizeof(ovs_be32);
> + ofpbuf_put(ofpacts, >value.ipv4, sizeof(ovs_be32));
> +}
> }
> }
>
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index cc63d82..38555db 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -57,7 +57,7 @@ struct gen_opts_map {
> #define DHCP_OPT_NIS_SERVER DHCP_OPTION("nis_server", 41, "ipv4")
> #define DHCP_OPT_NTP_SERVER DHCP_OPTION("ntp_server", 42, "ipv4")
> #define DHCP_OPT_SERVER_ID DHCP_OPTION("server_id", 54, "ipv4")
> -#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "ipv4")
> +#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "host_id")
>
> #define DHCP_OPT_CLASSLESS_STATIC_ROUTE \
> DHCP_OPTION("classless_static_route", 121, "static_routes")
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index eb78f31..ef08414 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11360,7 +11360,12 @@ check_and_add_supported_dhcp_opts_to_sb_db(struct
> northd_context *ctx)
> struct gen_opts_map *dhcp_opt =
> dhcp_opts_find(_opts_to_add, opt_row->name);
> if (dhcp_opt) {
> -hmap_remove(_opts_to_add, _opt->hmap_node);
> +if (!strcmp(dhcp_opt->type, opt_row->type) &&
> + dhcp_opt->code == opt_row->code) {
> +hmap_remove(_opts_to_add, _opt->hmap_node);
> +} else {
> +sbrec_dhcp_options_delete(opt_row);
> +}
> } else {
> sbrec_dhcp_options_delete(opt_row);
> }
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index c196dda..2ec729b 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> -"version": "2.8.0",
> -"cksum": "1643994484 21853",
> +"version": "2.8.1",
> +"cksum": "236203406 21905",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -217,7 +217,8 @@
> "type": {"key": {
> "type": "string",
> "enum": ["set", ["bool", "uint8", "uint16",
> "uint32",
> - "ipv4", "static_routes",
> "str"]],
> + "ipv4", "static_routes", "str",
> + "host_id"]],
> "isRoot": true},
> "DHCPv6_Options": {
> "columns": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 15204a8..208fb93 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3219,6 +3219,19 @@ tcp.flags = RST;
> Example. "name=host_name", "code=12", "type=str".
>
>
> +
> +value: host_id
> +
> +
> +This indicates that the value of the DHCP option is a host_id.
> +It can either be a host_name or an IP address.
> +
> +
> +
> +Example.
[ovs-dev] [PATCH v3 1/2 ovn] Fix the data type for DHCP option tftp_server (66)
From: Dhathri Purohith
Currently, DHCP option is of type ipv4. But, according to RFC 2132,
option 66 can be a hostname i.e, we should also accept string type.
In order to be backward compatible, a new type called "host_id" is
introduced, which accepts both ipv4 address and string. Type for DHCP
option 66 is changed to "host_id" instead of ipv4.
OVN northd code that updates the OVN southbound database is enhanced to
consider the change in the type and code for DHCP option, so that the
change in datatype is reflected.
Signed-off-by: Dhathri Purohith
---
lib/actions.c | 12
lib/ovn-l7.h| 2 +-
northd/ovn-northd.c | 7 ++-
ovn-sb.ovsschema| 7 ---
ovn-sb.xml | 13 +
tests/ovn.at| 6 ++
tests/test-ovn.c| 2 +-
7 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/lib/actions.c b/lib/actions.c
index c506151..f21be6d 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1978,6 +1978,10 @@ parse_gen_opt(struct action_context *ctx, struct
ovnact_gen_option *o,
return;
}
+if (!strcmp(o->option->type, "host_id")) {
+return;
+}
+
if (!strcmp(o->option->type, "str")) {
if (o->value.type != EXPR_C_STRING) {
lexer_error(ctx->lexer, "%s option %s requires string value.",
@@ -2305,6 +2309,14 @@ encode_put_dhcpv4_option(const struct ovnact_gen_option
*o,
} else if (!strcmp(o->option->type, "str")) {
opt_header[1] = strlen(c->string);
ofpbuf_put(ofpacts, c->string, opt_header[1]);
+} else if (!strcmp(o->option->type, "host_id")) {
+if (o->value.type == EXPR_C_STRING) {
+opt_header[1] = strlen(c->string);
+ofpbuf_put(ofpacts, c->string, opt_header[1]);
+} else {
+ opt_header[1] = sizeof(ovs_be32);
+ ofpbuf_put(ofpacts, >value.ipv4, sizeof(ovs_be32));
+}
}
}
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index cc63d82..38555db 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -57,7 +57,7 @@ struct gen_opts_map {
#define DHCP_OPT_NIS_SERVER DHCP_OPTION("nis_server", 41, "ipv4")
#define DHCP_OPT_NTP_SERVER DHCP_OPTION("ntp_server", 42, "ipv4")
#define DHCP_OPT_SERVER_ID DHCP_OPTION("server_id", 54, "ipv4")
-#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "ipv4")
+#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "host_id")
#define DHCP_OPT_CLASSLESS_STATIC_ROUTE \
DHCP_OPTION("classless_static_route", 121, "static_routes")
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index eb78f31..ef08414 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11360,7 +11360,12 @@ check_and_add_supported_dhcp_opts_to_sb_db(struct
northd_context *ctx)
struct gen_opts_map *dhcp_opt =
dhcp_opts_find(_opts_to_add, opt_row->name);
if (dhcp_opt) {
-hmap_remove(_opts_to_add, _opt->hmap_node);
+if (!strcmp(dhcp_opt->type, opt_row->type) &&
+ dhcp_opt->code == opt_row->code) {
+hmap_remove(_opts_to_add, _opt->hmap_node);
+} else {
+sbrec_dhcp_options_delete(opt_row);
+}
} else {
sbrec_dhcp_options_delete(opt_row);
}
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index c196dda..2ec729b 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
-"version": "2.8.0",
-"cksum": "1643994484 21853",
+"version": "2.8.1",
+"cksum": "236203406 21905",
"tables": {
"SB_Global": {
"columns": {
@@ -217,7 +217,8 @@
"type": {"key": {
"type": "string",
"enum": ["set", ["bool", "uint8", "uint16", "uint32",
- "ipv4", "static_routes", "str"]],
+ "ipv4", "static_routes", "str",
+ "host_id"]],
"isRoot": true},
"DHCPv6_Options": {
"columns": {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 15204a8..208fb93 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3219,6 +3219,19 @@ tcp.flags = RST;
Example. "name=host_name", "code=12", "type=str".
+
+value: host_id
+
+
+This indicates that the value of the DHCP option is a host_id.
+It can either be a host_name or an IP address.
+
+
+
+Example. "name=tftp_server", "code=66", "type=host_id".
+
+
+
diff --git a/tests/ovn.at b/tests/ovn.at
index 15b40ca..fa2592c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1212,6 +1212,12 @@ reg2[5] =
put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,m
reg0[15] =
