diff mbox

[ovs-dev,v1,RFC] ovn: Support native dhcp using 'continuations'

Message ID 570252CD.20208@redhat.com
State RFC
Headers show

Commit Message

Numan Siddique April 4, 2016, 11:41 a.m. UTC
To support native dhcp in ovn
 - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
 - A logical flow is added for each logical port to handle dhcp packets
   if the CMS has defined dhcp options in this column.

Eg.
action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
outport = inport; inport = ""; /* Allow sending out inport. */ output;)

 - ovn-controller will convert this logical flow to a packet-in OF flow with
   'pause' flag set. The dhcp options defined in 'dhcp_offer' action
   are stored in the 'userdata'

 - When the ovn-controller receives the packet-in, it would frame a new
   dhcp packet with the dhcp options set in the 'userdata' and resume
   the packet.

TODO: Test cases and updating the necessary documentation.

Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 lib/dhcp.h                |  13 +++++
 ovn/controller/lflow.c    |  27 ++++++++++
 ovn/controller/pinctrl.c  | 114 ++++++++++++++++++++++++++++++++++++++++-
 ovn/lib/actions.c         | 127 ++++++++++++++++++++++++++++++++++++++++++++--
 ovn/lib/actions.h         |   8 +++
 ovn/lib/expr.c            |  75 ++++++++++++++-------------
 ovn/lib/expr.h            |  57 +++++++++++++++++++++
 ovn/northd/ovn-northd.c   | 122 +++++++++++++++++++++++++++++++++++++++-----
 ovn/ovn-nb.ovsschema      |   7 ++-
 ovn/ovn-nb.xml            |   6 +++
 ovn/utilities/ovn-nbctl.c |  51 +++++++++++++++++++
 11 files changed, 554 insertions(+), 53 deletions(-)

Comments

Ryan Moats April 4, 2016, 12:46 p.m. UTC | #1
"dev" <dev-bounces@openvswitch.org> wrote on 04/04/2016 06:41:01 AM:

> From: Numan Siddique <nusiddiq@redhat.com>
> To: ovs dev <dev@openvswitch.org>
> Date: 04/04/2016 06:41 AM
> Subject: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using
> 'continuations'
> Sent by: "dev" <dev-bounces@openvswitch.org>
>
> To support native dhcp in ovn
>  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
>  - A logical flow is added for each logical port to handle dhcp packets
>    if the CMS has defined dhcp options in this column.
>
> Eg.
> action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
> ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> outport = inport; inport = ""; /* Allow sending out inport. */ output;)
>
>  - ovn-controller will convert this logical flow to a packet-in OF flow
with
>    'pause' flag set. The dhcp options defined in 'dhcp_offer' action
>    are stored in the 'userdata'
>
>  - When the ovn-controller receives the packet-in, it would frame a new
>    dhcp packet with the dhcp options set in the 'userdata' and resume
>    the packet.
>
> TODO: Test cases and updating the necessary documentation.
>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---

[snip]

First, let me say that I'm a big fan of this idea in general...

> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index bcad318..a175458 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -28,6 +28,7 @@
>  #include "packets.h"
>  #include "simap.h"
>
> +
>  VLOG_DEFINE_THIS_MODULE(lflow);
>
>  /* Symbol table. */
> @@ -35,6 +36,9 @@ VLOG_DEFINE_THIS_MODULE(lflow);
>  /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
>  static struct shash symtab;
>
> +/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
> +static struct shash dhcp_opt_symtab;
> +
>  static void
>  add_logical_register(struct shash *symtab, enum mf_field_id id)
>  {
> @@ -156,6 +160,27 @@ lflow_init(void)
>      expr_symtab_add_predicate(&symtab, "sctp", "ip.proto == 132");
>      expr_symtab_add_field(&symtab, "sctp.src", MFF_SCTP_SRC, "sctp",
false);
>      expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp",
false);
> +
> +    /* dhcp options */
> +    shash_init(&dhcp_opt_symtab);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
> +                                   DHCP_OPT_TYPE_STR);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab,
"ip_forward_enable", 19,
> +                                   DHCP_OPT_TYPE_BOOL);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> +                                   DHCP_OPT_TYPE_UINT16);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> +                                   DHCP_OPT_TYPE_UINT32);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> +                                   DHCP_OPT_TYPE_IP4);

but I'm rather uncomfortable with (a) using the bare option numbers
above (I think I'd rather see symbols and values defined in dhcp.h)
and (b) not supporting more/all of RFC 2132 (under this heading, isn't
opt 0 padding, rather than the offerip?)

Ryan
Russell Bryant April 4, 2016, 2:27 p.m. UTC | #2
On Mon, Apr 4, 2016 at 8:46 AM, Ryan Moats <rmoats@us.ibm.com> wrote:

>
> "dev" <dev-bounces@openvswitch.org> wrote on 04/04/2016 06:41:01 AM:
>
> > From: Numan Siddique <nusiddiq@redhat.com>
> > To: ovs dev <dev@openvswitch.org>
> > Date: 04/04/2016 06:41 AM
> > Subject: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using
> > 'continuations'
> > Sent by: "dev" <dev-bounces@openvswitch.org>
> >
> > To support native dhcp in ovn
> >  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
> >  - A logical flow is added for each logical port to handle dhcp packets
> >    if the CMS has defined dhcp options in this column.
> >
> > Eg.
> > action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> > server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> > netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
> > ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> > outport = inport; inport = ""; /* Allow sending out inport. */ output;)
> >
> >  - ovn-controller will convert this logical flow to a packet-in OF flow
> with
> >    'pause' flag set. The dhcp options defined in 'dhcp_offer' action
> >    are stored in the 'userdata'
> >
> >  - When the ovn-controller receives the packet-in, it would frame a new
> >    dhcp packet with the dhcp options set in the 'userdata' and resume
> >    the packet.
> >
> > TODO: Test cases and updating the necessary documentation.
> >
> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> > ---
>
> [snip]
>
> First, let me say that I'm a big fan of this idea in general...
>
> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > index bcad318..a175458 100644
> > --- a/ovn/controller/lflow.c
> > +++ b/ovn/controller/lflow.c
> > @@ -28,6 +28,7 @@
> >  #include "packets.h"
> >  #include "simap.h"
> >
> > +
> >  VLOG_DEFINE_THIS_MODULE(lflow);
> >
> >  /* Symbol table. */
> > @@ -35,6 +36,9 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> >  /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
> >  static struct shash symtab;
> >
> > +/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
> > +static struct shash dhcp_opt_symtab;
> > +
> >  static void
> >  add_logical_register(struct shash *symtab, enum mf_field_id id)
> >  {
> > @@ -156,6 +160,27 @@ lflow_init(void)
> >      expr_symtab_add_predicate(&symtab, "sctp", "ip.proto == 132");
> >      expr_symtab_add_field(&symtab, "sctp.src", MFF_SCTP_SRC, "sctp",
> false);
> >      expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp",
> false);
> > +
> > +    /* dhcp options */
> > +    shash_init(&dhcp_opt_symtab);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
> > +                                   DHCP_OPT_TYPE_STR);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab,
> "ip_forward_enable", 19,
> > +                                   DHCP_OPT_TYPE_BOOL);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> > +                                   DHCP_OPT_TYPE_UINT16);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> > +                                   DHCP_OPT_TYPE_UINT32);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> > +                                   DHCP_OPT_TYPE_IP4);
>
> but I'm rather uncomfortable with (a) using the bare option numbers
> above (I think I'd rather see symbols and values defined in dhcp.h)
> and (b) not supporting more/all of RFC 2132 (under this heading, isn't
> opt 0 padding, rather than the offerip?)
>

I agree that some constants would be an improvement.

Maybe I'm misreading your suggestion, but I definitely do not think we
should be trying to implement a "full featured" DHCP server in OVN.  The
intention here is "just enough" DHCP.  We only need minimal support to be
able to issue DHCP responses based on statically defined IP/MAC mappings.
Ryan Moats April 4, 2016, 2:40 p.m. UTC | #3
Russell Bryant <russell@ovn.org> wrote on 04/04/2016 09:27:30 AM:

> From: Russell Bryant <russell@ovn.org>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: Numan Siddique <nusiddiq@redhat.com>, ovs dev <dev@openvswitch.org>
> Date: 04/04/2016 09:28 AM
> Subject: Re: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp
> using'continuations'
>
> On Mon, Apr 4, 2016 at 8:46 AM, Ryan Moats <rmoats@us.ibm.com> wrote:
>
> "dev" <dev-bounces@openvswitch.org> wrote on 04/04/2016 06:41:01 AM:
>
> > From: Numan Siddique <nusiddiq@redhat.com>
> > To: ovs dev <dev@openvswitch.org>
> > Date: 04/04/2016 06:41 AM
> > Subject: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using
> > 'continuations'
> > Sent by: "dev" <dev-bounces@openvswitch.org>
> >
> > To support native dhcp in ovn
> >  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
> >  - A logical flow is added for each logical port to handle dhcp packets
> >    if the CMS has defined dhcp options in this column.
> >
> > Eg.
> > action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> > server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> > netmask = 255.255.255.0); eth.dst = eth.src; eth.src =
00:00:00:00:00:03;
> > ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> > outport = inport; inport = ""; /* Allow sending out inport. */ output;)
> >
> >  - ovn-controller will convert this logical flow to a packet-in OF flow
> with
> >    'pause' flag set. The dhcp options defined in 'dhcp_offer' action
> >    are stored in the 'userdata'
> >
> >  - When the ovn-controller receives the packet-in, it would frame a new
> >    dhcp packet with the dhcp options set in the 'userdata' and resume
> >    the packet.
> >
> > TODO: Test cases and updating the necessary documentation.
> >
> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> > ---
>
> [snip]
>
> First, let me say that I'm a big fan of this idea in general...
>
> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > index bcad318..a175458 100644
> > --- a/ovn/controller/lflow.c
> > +++ b/ovn/controller/lflow.c
> > @@ -28,6 +28,7 @@
> >  #include "packets.h"
> >  #include "simap.h"
> >
> > +
> >  VLOG_DEFINE_THIS_MODULE(lflow);
> >
> >  /* Symbol table. */
> > @@ -35,6 +36,9 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> >  /* Contains "struct expr_symbol"s for fields supported by OVN lflows.
*/
> >  static struct shash symtab;
> >
> > +/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
> > +static struct shash dhcp_opt_symtab;
> > +
> >  static void
> >  add_logical_register(struct shash *symtab, enum mf_field_id id)
> >  {
> > @@ -156,6 +160,27 @@ lflow_init(void)
> >      expr_symtab_add_predicate(&symtab, "sctp", "ip.proto == 132");
> >      expr_symtab_add_field(&symtab, "sctp.src", MFF_SCTP_SRC, "sctp",
> false);
> >      expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp",
> false);
> > +
> > +    /* dhcp options */
> > +    shash_init(&dhcp_opt_symtab);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name",
15,
> > +                                   DHCP_OPT_TYPE_STR);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab,
> "ip_forward_enable", 19,
> > +                                   DHCP_OPT_TYPE_BOOL);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> > +                                   DHCP_OPT_TYPE_UINT16);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> > +                                   DHCP_OPT_TYPE_UINT32);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> > +                                   DHCP_OPT_TYPE_IP4);

> but I'm rather uncomfortable with (a) using the bare option numbers
> above (I think I'd rather see symbols and values defined in dhcp.h)
> and (b) not supporting more/all of RFC 2132 (under this heading, isn't
> opt 0 padding, rather than the offerip?)
>
> I agree that some constants would be an improvement.
>
> Maybe I'm misreading your suggestion, but I definitely do not think
> we should be trying to implement a "full featured" DHCP server in
> OVN.  The intention here is "just enough" DHCP.  We only need
> minimal support to be able to issue DHCP responses based on
> statically defined IP/MAC mappings.

I certainly don't want a full DHCP server in OVN and so I'd like to
see something somewhere that *says* this isn't a full featured DHCP
as it provides some minimal set of functionality (and then listing
the functions).  However, my sad experience with providing a minimal
set of functionality is that it becomes a slippery slope of "just
one more thing" (for example, what about static routes?).

Ryan
Ramu Ramamurthy April 4, 2016, 6:47 p.m. UTC | #4
> @@ -89,10 +89,11 @@ enum ovn_stage {
>      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_L2,    0, "ls_in_port_sec_l2")     \
>      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_IP,    1, "ls_in_port_sec_ip")     \
>      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,    2, "ls_in_port_sec_nd")     \
> -    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        3, "ls_in_pre_acl")      \
> -    PIPELINE_STAGE(SWITCH, IN,  ACL,            4, "ls_in_acl")          \
> -    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        5, "ls_in_arp_rsp")      \
> -    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        6, "ls_in_l2_lkup")      \
> +    PIPELINE_STAGE(SWITCH, IN,  DHCP,           3, "ls_in_dhcp")     \
> +    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        4, "ls_in_pre_acl")      \
> +    PIPELINE_STAGE(SWITCH, IN,  ACL,            5, "ls_in_acl")          \
> +    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        6, "ls_in_arp_rsp")      \
> +    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        7, "ls_in_l2_lkup")      \
>                                                                        \

Would it make sense to put DHCP after ACL instead of before - so,
some control is
provided on the handling of DHCP packets via acl rules. For instance,
OpenStack programs these DHCP ACL
rules currently allowing client->server communication - and the
usecase would be if i want to drop
all client->server DHCP traffic for a rogue VM.

table=2(       ls_in_acl), priority= 2002, match=(inport ==
"55c0912f-f7aa-4318-82f1-6118032839e3" && ip4 && (ip4.dst ==
255.255.255.255 || ip4.dst == 10.10.0.0/16) && udp && udp.src == 68 &&
udp.dst == 67), action=(ct_commit; next;)
Numan Siddique April 5, 2016, 11:38 a.m. UTC | #5
On Mon, Apr 4, 2016 at 8:10 PM, Ryan Moats <rmoats@us.ibm.com> wrote:

> Russell Bryant <russell@ovn.org> wrote on 04/04/2016 09:27:30 AM:
>
> > From: Russell Bryant <russell@ovn.org>
> > To: Ryan Moats/Omaha/IBM@IBMUS
> > Cc: Numan Siddique <nusiddiq@redhat.com>, ovs dev <dev@openvswitch.org>
> > Date: 04/04/2016 09:28 AM
> > Subject: Re: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp
> > using'continuations'
>
> >
> > On Mon, Apr 4, 2016 at 8:46 AM, Ryan Moats <rmoats@us.ibm.com> wrote:
> >
> > "dev" <dev-bounces@openvswitch.org> wrote on 04/04/2016 06:41:01 AM:
> >
> > > From: Numan Siddique <nusiddiq@redhat.com>
> > > To: ovs dev <dev@openvswitch.org>
> > > Date: 04/04/2016 06:41 AM
> > > Subject: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using
> > > 'continuations'
> > > Sent by: "dev" <dev-bounces@openvswitch.org>
> > >
> > > To support native dhcp in ovn
> > >  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
> > >  - A logical flow is added for each logical port to handle dhcp packets
> > >    if the CMS has defined dhcp options in this column.
> > >
> > > Eg.
> > > action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> > > server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> > > netmask = 255.255.255.0); eth.dst = eth.src; eth.src =
> 00:00:00:00:00:03;
> > > ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> > > outport = inport; inport = ""; /* Allow sending out inport. */ output;)
> > >
> > >  - ovn-controller will convert this logical flow to a packet-in OF flow
> > with
> > >    'pause' flag set. The dhcp options defined in 'dhcp_offer' action
> > >    are stored in the 'userdata'
> > >
> > >  - When the ovn-controller receives the packet-in, it would frame a new
> > >    dhcp packet with the dhcp options set in the 'userdata' and resume
> > >    the packet.
> > >
> > > TODO: Test cases and updating the necessary documentation.
> > >
> > > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> > > ---
> >
> > [snip]
> >
> > First, let me say that I'm a big fan of this idea in general...
> >
> > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > > index bcad318..a175458 100644
> > > --- a/ovn/controller/lflow.c
> > > +++ b/ovn/controller/lflow.c
> > > @@ -28,6 +28,7 @@
> > >  #include "packets.h"
> > >  #include "simap.h"
> > >
> > > +
> > >  VLOG_DEFINE_THIS_MODULE(lflow);
> > >
> > >  /* Symbol table. */
> > > @@ -35,6 +36,9 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> > >  /* Contains "struct expr_symbol"s for fields supported by OVN lflows.
> */
> > >  static struct shash symtab;
> > >
> > > +/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
> > > +static struct shash dhcp_opt_symtab;
> > > +
> > >  static void
> > >  add_logical_register(struct shash *symtab, enum mf_field_id id)
> > >  {
> > > @@ -156,6 +160,27 @@ lflow_init(void)
> > >      expr_symtab_add_predicate(&symtab, "sctp", "ip.proto == 132");
> > >      expr_symtab_add_field(&symtab, "sctp.src", MFF_SCTP_SRC, "sctp",
> > false);
> > >      expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp",
> > false);
> > > +
> > > +    /* dhcp options */
> > > +    shash_init(&dhcp_opt_symtab);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> > > +                                   DHCP_OPT_TYPE_IP4);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> > > +                                   DHCP_OPT_TYPE_IP4);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> > > +                                   DHCP_OPT_TYPE_IP4);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> > > +                                   DHCP_OPT_TYPE_IP4);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name",
> 15,
> > > +                                   DHCP_OPT_TYPE_STR);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab,
> > "ip_forward_enable", 19,
> > > +                                   DHCP_OPT_TYPE_BOOL);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> > > +                                   DHCP_OPT_TYPE_UINT16);
> > > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> > > +                                   DHCP_OPT_TYPE_UINT32);
> > > +
> ​​
> dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> > > +                                   DHCP_OPT_TYPE_IP4);
>
> > but I'm rather uncomfortable with (a) using the bare option numbers
> > above (I think I'd rather see symbols and values defined in dhcp.h)
> > and (b) not supporting more/all of RFC 2132 (under this heading, isn't
> > opt 0 padding, rather than the offerip?)
> >
>

​Thanks Ryan for pointing this out. I will define them in dhcp.h

'offerip' is actually not a dhcp option. I am using opt 0 since CMS would
not define
this dhcp option.

​The userdata would be of the format

---------------------------------------------------------------------------------------
|ACTION_OPCODE_DHCP_OFFER (8 bytes) | offer_ip (4 bytes) | <dhcp options>
           |
 --------------------------------------------------------------------------------------
 ​


​


> > I agree that some constants would be an improvement.
> >
> > Maybe I'm misreading your suggestion, but I definitely do not think
> > we should be trying to implement a "full featured" DHCP server in
> > OVN.  The intention here is "just enough" DHCP.  We only need
> > minimal support to be able to issue DHCP responses based on
> > statically defined IP/MAC mappings.
>
> I certainly don't want a full DHCP server in OVN and so I'd like to
> see something somewhere that *says* this isn't a full featured DHCP
> as it provides some minimal set of functionality (and then listing
> the functions).  However, my sad experience with providing a minimal
> set of functionality is that it becomes a slippery slope of "just
> one more thing" (for example, what about static routes?).
>

​Even I was thinking to identify a minimal set of dhcp options and support
them.
If the requirement arises for more dhcp options, we can raise a bug. In
order to support
​more dhcp options we just need to define them in 'dhcp_opt_symtab'.
In the next version, I will add more dhcp options which are more likely to
be used, although I am not sure how to select them :)


Thanks
Numan


>
> Ryan
>
>
Numan Siddique April 5, 2016, 11:52 a.m. UTC | #6
On Tue, Apr 5, 2016 at 12:17 AM, Ramu Ramamurthy <ramu.ramamurthy@gmail.com>
wrote:

> > @@ -89,10 +89,11 @@ enum ovn_stage {
> >      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_L2,    0,
> "ls_in_port_sec_l2")     \
> >      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_IP,    1,
> "ls_in_port_sec_ip")     \
> >      PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,    2,
> "ls_in_port_sec_nd")     \
> > -    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        3, "ls_in_pre_acl")
>   \
> > -    PIPELINE_STAGE(SWITCH, IN,  ACL,            4, "ls_in_acl")
>   \
> > -    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        5, "ls_in_arp_rsp")
>   \
> > -    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        6, "ls_in_l2_lkup")
>   \
> > +    PIPELINE_STAGE(SWITCH, IN,  DHCP,           3, "ls_in_dhcp")     \
> > +    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        4, "ls_in_pre_acl")
>   \
> > +    PIPELINE_STAGE(SWITCH, IN,  ACL,            5, "ls_in_acl")
>   \
> > +    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        6, "ls_in_arp_rsp")
>   \
> > +    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        7, "ls_in_l2_lkup")
>   \
> >                                                                        \
>
> Would it make sense to put DHCP after ACL instead of before - so,
> some control is
> provided on the handling of DHCP packets via acl rules. For instance,
> OpenStack programs these DHCP ACL
> rules currently allowing client->server communication - and the
> usecase would be if i want to drop
> all client->server DHCP traffic for a rogue VM.
>
> table=2(       ls_in_acl), priority= 2002, match=(inport ==
> "55c0912f-f7aa-4318-82f1-6118032839e3" && ip4 && (ip4.dst ==
> 255.255.255.255 || ip4.dst == 10.10.0.0/16) && udp && udp.src == 68 &&
> udp.dst == 67), action=(ct_commit; next;)
>

​Since the dhcp traffic is handled by the ovn-controller, I thought there
is no need for
the ACL checks. I do not have any preference as such. I will test this out
once.

Thanks
Numan
Ramu Ramamurthy April 13, 2016, 6:52 p.m. UTC | #7
> +    /* dhcp options */
> +    shash_init(&dhcp_opt_symtab);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> +                                   DHCP_OPT_TYPE_IP4);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
> +                                   DHCP_OPT_TYPE_STR);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "ip_forward_enable", 19,
> +                                   DHCP_OPT_TYPE_BOOL);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> +                                   DHCP_OPT_TYPE_UINT16);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> +                                   DHCP_OPT_TYPE_UINT32);
> +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> +                                   DHCP_OPT_TYPE_IP4);
>  }

Numan, DHCP static-route option (249) is used in the DHCP-offer to
enable VM Metadata access.
Could you consider adding support for that option - Or I can help to
add that option.
Ben Pfaff April 13, 2016, 11:37 p.m. UTC | #8
On Mon, Apr 04, 2016 at 05:11:01PM +0530, Numan Siddique wrote:
> To support native dhcp in ovn
>  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
>  - A logical flow is added for each logical port to handle dhcp packets
>    if the CMS has defined dhcp options in this column.
> 
> Eg.
> action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
> ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> outport = inport; inport = ""; /* Allow sending out inport. */ output;)
> 
>  - ovn-controller will convert this logical flow to a packet-in OF flow with
>    'pause' flag set. The dhcp options defined in 'dhcp_offer' action
>    are stored in the 'userdata'
> 
>  - When the ovn-controller receives the packet-in, it would frame a new
>    dhcp packet with the dhcp options set in the 'userdata' and resume
>    the packet.
> 
> TODO: Test cases and updating the necessary documentation.
> 
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>

Thanks for working on this!  I am so much happier with this than with
the previous generation of DHCP support.  It is much more in line with
the "OVN philosophy" if I can be forgiven for saying that.

I get some warnings from Clang:

    ../ovn/controller/pinctrl.c:203:6: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
    ../ovn/controller/pinctrl.c:217:7: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
    ../ovn/controller/pinctrl.c:225:7: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
    ../ovn/controller/pinctrl.c:272:28: error: cast from 'char *' to 'struct ip_header *' increases required alignment from 1 to 2 [-Werror,-Wcast-align]
    ../ovn/controller/pinctrl.c:273:30: error: cast from 'char *' to 'struct udp_header *' increases required alignment from 1 to 2 [-Werror,-Wcast-align]
    ../ovn/controller/pinctrl.c:275:37: error: cast from 'char *' to 'struct dhcp_header *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]

and from sparse:

    ../ovn/controller/pinctrl.c:203:24: warning: incorrect type in assignment (different base types)
    ../ovn/controller/pinctrl.c:203:24:    expected unsigned int [unsigned] [usertype] <noident>
    ../ovn/controller/pinctrl.c:203:24:    got restricted ovs_be32
    ../ovn/controller/pinctrl.c:284:37: warning: incorrect type in argument 1 (different base types)
    ../ovn/controller/pinctrl.c:284:37:    expected restricted ofp_port_t [usertype] ofp_port
    ../ovn/controller/pinctrl.c:284:37:    got int [signed] udp_len
    ../ovn/controller/pinctrl.c:286:53: warning: incorrect type in argument 1 (different base types)
    ../ovn/controller/pinctrl.c:286:53:    expected restricted ofp_port_t [usertype] ofp_port
    ../ovn/controller/pinctrl.c:286:53:    got int

This is less code than I expected; that's great!

This is an RFC so I won't nitpick.

I do have one bigger idea: what if the collection of DHCP options were
to be read from a new table in the southbound database?  This would make
it easy to add and remove support for particular options by updating
just ovn-northd.  It would solve the
Ben Pfaff April 13, 2016, 11:38 p.m. UTC | #9
On Tue, Apr 05, 2016 at 05:08:00PM +0530, Numan Siddique wrote:
> ​Even I was thinking to identify a minimal set of dhcp options and
> support them.  If the requirement arises for more dhcp options, we can
> raise a bug. In order to support ​more dhcp options we just need to
> define them in 'dhcp_opt_symtab'.  In the next version, I will add
> more dhcp options which are more likely to be used, although I am not
> sure how to select them :)

It wouldn't be hard to have a new OVN_Southbound table DHCP_Option with
columns "name", "code", and "type", and that would give us tons of
flexibility.
Numan Siddique April 14, 2016, 10:30 a.m. UTC | #10
On Thu, Apr 14, 2016 at 12:22 AM, Ramu Ramamurthy <ramu.ramamurthy@gmail.com
> wrote:

> > +    /* dhcp options */
> > +    shash_init(&dhcp_opt_symtab);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> > +                                   DHCP_OPT_TYPE_IP4);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
> > +                                   DHCP_OPT_TYPE_STR);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab,
> "ip_forward_enable", 19,
> > +                                   DHCP_OPT_TYPE_BOOL);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> > +                                   DHCP_OPT_TYPE_UINT16);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> > +                                   DHCP_OPT_TYPE_UINT32);
> > +    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> > +                                   DHCP_OPT_TYPE_IP4);
> >  }
>
> Numan, DHCP static-route option (249) is used in the DHCP-offer to
> enable VM Metadata access.
> Could you consider adding support for that option - Or I can help to
> add that option.
>

​I have already started into supporting dhcp option 121 for static routes.
https://tools.ietf.org/html/rfc3442

I think we can support both. ​


​Thanks
Numan
​
diff mbox

Patch

diff --git a/lib/dhcp.h b/lib/dhcp.h
index 6f97298..c9537e7 100644
--- a/lib/dhcp.h
+++ b/lib/dhcp.h
@@ -25,6 +25,19 @@ 
 #define DHCP_SERVER_PORT        67       /* Port used by DHCP server. */
 #define DHCP_CLIENT_PORT        68       /* Port used by DHCP client. */
 
+#define DHCP_MAGIC_COOKIE (uint32_t)0x63825363
+
+#define DHCP_OP_REQUEST    ((uint8_t)1)
+#define DHCP_OP_REPLY      ((uint8_t)2)
+
+#define DHCP_MSG_DISCOVER  ((uint8_t)1)
+#define DHCP_MSG_OFFER     ((uint8_t)2)
+#define DHCP_MSG_REQUEST   ((uint8_t)3)
+#define DHCP_MSG_ACK       ((uint8_t)5)
+
+#define DHCP_OPT_MSG_TYPE  ((uint8_t)53)
+#define DHCP_OPT_END       ((uint8_t)255)
+
 #define DHCP_HEADER_LEN 236
 struct dhcp_header {
     uint8_t op;                 /* DHCP_BOOTREQUEST or DHCP_BOOTREPLY. */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index bcad318..a175458 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -28,6 +28,7 @@ 
 #include "packets.h"
 #include "simap.h"
 
+
 VLOG_DEFINE_THIS_MODULE(lflow);
 
 /* Symbol table. */
@@ -35,6 +36,9 @@  VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
+/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
+static struct shash dhcp_opt_symtab;
+
 static void
 add_logical_register(struct shash *symtab, enum mf_field_id id)
 {
@@ -156,6 +160,27 @@  lflow_init(void)
     expr_symtab_add_predicate(&symtab, "sctp", "ip.proto == 132");
     expr_symtab_add_field(&symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false);
     expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
+
+    /* dhcp options */
+    shash_init(&dhcp_opt_symtab);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
+                                   DHCP_OPT_TYPE_IP4);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
+                                   DHCP_OPT_TYPE_IP4);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
+                                   DHCP_OPT_TYPE_IP4);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
+                                   DHCP_OPT_TYPE_IP4);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
+                                   DHCP_OPT_TYPE_STR);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "ip_forward_enable", 19,
+                                   DHCP_OPT_TYPE_BOOL);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
+                                   DHCP_OPT_TYPE_UINT16);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
+                                   DHCP_OPT_TYPE_UINT32);
+    dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
+                                   DHCP_OPT_TYPE_IP4);
 }
 
 struct lookup_port_aux {
@@ -280,6 +305,7 @@  add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
         };
         struct action_params ap = {
             .symtab = &symtab,
+            .dhcp_opt_symtab = &dhcp_opt_symtab,
             .lookup_port = lookup_port_cb,
             .aux = &aux,
             .ct_zones = ct_zones,
@@ -443,4 +469,5 @@  void
 lflow_destroy(void)
 {
     expr_symtab_destroy(&symtab);
+    dhcp_opt_expr_symtab_destroy(&dhcp_opt_symtab);
 }
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 3fcab99..a3df5d8 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -17,9 +17,12 @@ 
 
 #include "pinctrl.h"
 
+
 #include "coverage.h"
+#include "csum.h"
 #include "dirs.h"
 #include "dp-packet.h"
+#include "lib/dhcp.h"
 #include "lport.h"
 #include "ofp-actions.h"
 #include "ovn/lib/actions.h"
@@ -192,13 +195,118 @@  exit:
 }
 
 static void
+compose_dhcp_options(struct ofpbuf *userdata, uint8_t *options,
+                     unsigned int *options_len, uint8_t msg_type)
+{
+    uint8_t *opts = options;
+
+    *(uint32_t *) opts = htonl(DHCP_MAGIC_COOKIE);
+    opts += (sizeof (uint32_t));
+
+    /* Dhcp option - type */
+    opts[0] = (uint8_t) DHCP_OPT_MSG_TYPE;
+    opts[1] = (uint8_t) 1;
+    opts[2] = msg_type;
+
+    opts += 3;
+
+    memcpy(opts, userdata->data, userdata->size);
+    opts += userdata->size;
+
+    /* Padding */
+    *((uint32_t *) opts) = 0;
+    opts += 4;
+
+    /* End */
+    opts[0] = DHCP_OPT_END;
+    opts += 1;
+
+    /* Padding */
+    *((uint32_t *) opts) = 0;
+    opts += 4;
+
+    *options_len = (opts - options);
+}
+
+static void
+pinctl_handle_dhcp_offer(struct dp_packet *pkt, struct ofputil_packet_in *pin,
+                         struct ofpbuf *userdata, struct ofpbuf *continuation)
+{
+    /* Compose a DHCP response packet. */
+    uint8_t options[256];
+    unsigned int options_len = 0;
+    memset(options, 0, sizeof(options));
+
+    ovs_be32 *offer_ip = ofpbuf_pull(userdata, sizeof *offer_ip);
+    const uint8_t *payload = dp_packet_get_udp_payload(pkt);
+    payload = (payload + sizeof (struct dhcp_header) + sizeof(uint32_t));
+
+    if (payload[0] != DHCP_OPT_MSG_TYPE) {
+        return;
+    }
+
+    uint8_t msg_type;
+    if (payload[2] == DHCP_MSG_DISCOVER) {
+        msg_type = DHCP_MSG_OFFER;
+    } else if(payload[2] == DHCP_MSG_REQUEST) {
+        msg_type = DHCP_MSG_ACK;
+    } else {
+        return;
+    }
+
+    compose_dhcp_options(userdata, options, &options_len, msg_type);
+
+    size_t new_packet_len = ETH_HEADER_LEN + IP_HEADER_LEN + \
+                            UDP_HEADER_LEN + DHCP_HEADER_LEN + options_len;
+
+    struct dp_packet packet;
+    dp_packet_init(&packet, new_packet_len);
+    dp_packet_clear(&packet);
+    dp_packet_prealloc_tailroom(&packet, new_packet_len);
+
+    dp_packet_put(
+        &packet, pin->packet,
+        new_packet_len < pin->packet_len ? new_packet_len : pin->packet_len);
+    dp_packet_set_size(&packet, new_packet_len);
+
+    struct ip_header *ip = (struct ip_header *)((char *)dp_packet_data(&packet) + ETH_HEADER_LEN);
+    struct udp_header *udp = (struct udp_header *)((char *)ip + IP_HEADER_LEN);
+
+    struct dhcp_header *dhcp_data = (struct dhcp_header *)((char *)udp + UDP_HEADER_LEN);
+    dhcp_data->op = DHCP_OP_REPLY;
+    dhcp_data->yiaddr = *offer_ip;
+
+    char *dhcp_opt = ((char *)dhcp_data + sizeof (*dhcp_data));
+    memcpy(dhcp_opt, options, options_len);
+
+    int udp_len = sizeof (*dhcp_data) + options_len + UDP_HEADER_LEN;
+
+    udp->udp_len = htons(ofp_to_u16(udp_len));
+    ip->ip_tos = 0;
+    ip->ip_tot_len = htons(ofp_to_u16(IP_HEADER_LEN + udp_len));
+    udp->udp_csum = 0;
+    ip->ip_csum = 0;
+    ip->ip_csum = csum(ip, sizeof *ip);
+
+    enum ofp_version version = rconn_get_version(swconn);
+    enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+    pin->packet = dp_packet_data(&packet);
+    pin->packet_len = dp_packet_size(&packet);
+
+    queue_msg(ofputil_encode_resume(pin, continuation, proto));
+    dp_packet_uninit(&packet);
+}
+
+static void
 process_packet_in(const struct ofp_header *msg)
 {
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
     struct ofputil_packet_in pin;
+    struct ofpbuf continuation;
     enum ofperr error = ofputil_decode_packet_in(msg, true, &pin,
-                                                 NULL, NULL, NULL);
+                                                 NULL, NULL, &continuation);
+
     if (error) {
         VLOG_WARN_RL(&rl, "error decoding packet-in: %s",
                      ofperr_to_string(error));
@@ -230,6 +338,9 @@  process_packet_in(const struct ofp_header *msg)
         pinctrl_handle_put_arp(&pin.flow_metadata.flow, &headers);
         break;
 
+    case ACTION_OPCODE_DHCP_OFFER:
+        pinctl_handle_dhcp_offer(&packet, &pin, &userdata, &continuation);
+        break;
     default:
         VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32,
                      ntohl(ah->opcode));
@@ -240,6 +351,7 @@  process_packet_in(const struct ofp_header *msg)
 static void
 pinctrl_recv(const struct ofp_header *oh, enum ofptype type)
 {
+
     if (type == OFPTYPE_ECHO_REQUEST) {
         queue_msg(make_echo_reply(oh));
     } else if (type == OFPTYPE_GET_CONFIG_REPLY) {
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 44957c7..6bb52cc 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -25,6 +25,7 @@ 
 #include "logical-fields.h"
 #include "ofp-actions.h"
 #include "openvswitch/ofpbuf.h"
+#include "shash.h"
 #include "simap.h"
 
 /* Context maintained during actions_parse(). */
@@ -185,13 +186,15 @@  add_prerequisite(struct action_context *ctx, const char *prerequisite)
 }
 
 static size_t
-start_controller_op(struct ofpbuf *ofpacts, enum action_opcode opcode)
+start_controller_op(struct ofpbuf *ofpacts, enum action_opcode opcode,
+                    bool pause)
 {
     size_t ofs = ofpacts->size;
 
     struct ofpact_controller *oc = ofpact_put_CONTROLLER(ofpacts);
     oc->max_len = UINT16_MAX;
     oc->reason = OFPR_ACTION;
+    oc->pause = pause;
 
     struct action_header ah = { .opcode = htonl(opcode) };
     ofpbuf_put(ofpacts, &ah, sizeof ah);
@@ -211,7 +214,7 @@  finish_controller_op(struct ofpbuf *ofpacts, size_t ofs)
 static void
 put_controller_op(struct ofpbuf *ofpacts, enum action_opcode opcode)
 {
-    size_t ofs = start_controller_op(ofpacts, opcode);
+    size_t ofs = start_controller_op(ofpacts, opcode, false);
     finish_controller_op(ofpacts, ofs);
 }
 
@@ -245,7 +248,9 @@  parse_arp_action(struct action_context *ctx)
      * converted to OpenFlow, as its userdata.  ovn-controller will convert the
      * packet to an ARP and then send the packet and actions back to the switch
      * inside an OFPT_PACKET_OUT message. */
-    size_t oc_offset = start_controller_op(ctx->ofpacts, ACTION_OPCODE_ARP);
+    /* controller. */
+    size_t oc_offset = start_controller_op(ctx->ofpacts, ACTION_OPCODE_ARP,
+                                           false);
     ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
                                  ctx->ofpacts, OFP13_VERSION);
     finish_controller_op(ctx->ofpacts, oc_offset);
@@ -260,6 +265,120 @@  parse_arp_action(struct action_context *ctx)
 }
 
 static bool
+parse_dhcp_option(struct action_context *ctx, struct ofpbuf *dhcp_opts)
+{
+    if (ctx->lexer->token.type != LEX_T_ID) {
+        action_syntax_error(ctx, NULL);
+        return false;
+    }
+
+    enum lex_type lookahead = lexer_lookahead(ctx->lexer);
+    if (lookahead != LEX_T_EQUALS) {
+        action_syntax_error(ctx, NULL);
+        return false;
+    }
+
+    const struct expr_symbol_dhcp_opts *symbol = shash_find_data(
+        ctx->ap->dhcp_opt_symtab, ctx->lexer->token.s);
+
+    if (!symbol) {
+        action_syntax_error(ctx, "expecting valid dhcp option.");
+        return false;
+    }
+
+    lexer_get(ctx->lexer);
+    lexer_get(ctx->lexer);
+
+    struct expr_constant_set cs;
+    memset(&cs, 0, sizeof(struct expr_constant_set));
+    if (!expr_parse_constant_set(ctx->lexer, ctx->ap->dhcp_opt_symtab, &cs)) {
+        action_syntax_error(ctx, "invalid dhcp option values");
+        return false;
+    }
+
+    if (!lexer_match(ctx->lexer, LEX_T_COMMA) && (
+        ctx->lexer->token.type != LEX_T_RPAREN)) {
+        action_syntax_error(ctx, NULL);
+        return false;
+    }
+
+
+    if (symbol->opt_code == 0) {
+        /* offer-ip */
+        ofpbuf_push(dhcp_opts, &cs.values[0].value.ipv4, sizeof(ovs_be32));
+        goto exit;
+    }
+
+    uint8_t *opt_header = ofpbuf_put_uninit(dhcp_opts, 2);
+    opt_header[0] = symbol->opt_code;
+
+
+    switch(symbol->opt_type) {
+    case DHCP_OPT_TYPE_BOOL:
+    case DHCP_OPT_TYPE_UINT8:
+        opt_header[1] = 1;
+        uint8_t val = cs.values[0].value.u8[127];
+        ofpbuf_put(dhcp_opts, &val, 1);
+        break;
+
+    case DHCP_OPT_TYPE_UINT16:
+        opt_header[1] = 2;
+        ofpbuf_put(dhcp_opts, &cs.values[0].value.be16[63], 2);
+        break;
+
+    case DHCP_OPT_TYPE_UINT32:
+        opt_header[1] = 4;
+        ofpbuf_put(dhcp_opts, &cs.values[0].value.be32[31], 4);
+        break;
+
+    case DHCP_OPT_TYPE_IP4:
+        opt_header[1] = 0;
+        for (size_t i = 0; i < cs.n_values; i++) {
+            ofpbuf_put(dhcp_opts, &cs.values[i].value.ipv4, sizeof(ovs_be32));
+            opt_header[1] += sizeof(ovs_be32);
+        }
+        break;
+
+    case DHCP_OPT_TYPE_STR:
+        opt_header[1] = strlen(cs.values[0].string);
+        ofpbuf_put(dhcp_opts, cs.values[0].string, opt_header[1]);
+        break;
+    }
+
+exit:
+    expr_constant_set_destroy(&cs);
+    return true;
+}
+
+static void
+parse_dhcp_offer_action(struct action_context *ctx)
+{
+    if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+        action_syntax_error(ctx, "expecting `('");
+        return;
+    }
+
+    uint64_t dhcp_opts_stub[1024 / 8];
+    struct ofpbuf dhcp_opts = OFPBUF_STUB_INITIALIZER(dhcp_opts_stub);
+
+    /* Parse inner actions. */
+    while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+        if (!parse_dhcp_option(ctx, &dhcp_opts)) {
+            return;
+        }
+    }
+
+    /* controller. */
+    size_t oc_offset = start_controller_op(ctx->ofpacts,
+                                           ACTION_OPCODE_DHCP_OFFER, true);
+    ofpbuf_put(ctx->ofpacts, dhcp_opts.data, dhcp_opts.size);
+    finish_controller_op(ctx->ofpacts, oc_offset);
+
+    /* Free memory. */
+    ofpbuf_uninit(&dhcp_opts);
+}
+
+static bool
 action_force_match(struct action_context *ctx, enum lex_type t)
 {
     if (lexer_match(ctx->lexer, t)) {
@@ -473,6 +592,8 @@  parse_action(struct action_context *ctx)
         parse_get_arp_action(ctx);
     } else if (lexer_match_id(ctx->lexer, "put_arp")) {
         parse_put_arp_action(ctx);
+    } else if (lexer_match_id(ctx->lexer, "dhcp_offer")) {
+        parse_dhcp_offer_action(ctx);
     } else {
         action_syntax_error(ctx, "expecting action");
     }
diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index 29af06f..782c5ba 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -44,6 +44,11 @@  enum action_opcode {
      *     MFF_ETH_SRC = mac
      */
     ACTION_OPCODE_PUT_ARP,
+
+    /* "dhcp_offer(...dhcp actions ...}".
+     *
+     */
+    ACTION_OPCODE_DHCP_OFFER,
 };
 
 /* Header. */
@@ -58,6 +63,9 @@  struct action_params {
      * expr_parse()). */
     const struct shash *symtab;
 
+    /* A table of "struct dhcp_opt_expr_symbol"s to support 'dhcp_offer' */
+    const struct shash *dhcp_opt_symtab;
+
     /* Looks up logical port 'port_name'.  If found, stores its port number in
      * '*portp' and returns true; otherwise, returns false. */
     bool (*lookup_port)(const void *aux, const char *port_name,
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index aae5df6..5e1db16 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -404,39 +404,6 @@  expr_print(const struct expr *e)
 
 /* Parsing. */
 
-/* Type of a "union expr_constant" or "struct expr_constant_set". */
-enum expr_constant_type {
-    EXPR_C_INTEGER,
-    EXPR_C_STRING
-};
-
-/* A string or integer constant (one must know which from context). */
-union expr_constant {
-    /* Integer constant.
-     *
-     * The width of a constant isn't always clear, e.g. if you write "1",
-     * there's no way to tell whether you mean for that to be a 1-bit constant
-     * or a 128-bit constant or somewhere in between. */
-    struct {
-        union mf_subvalue value;
-        union mf_subvalue mask; /* Only initialized if 'masked'. */
-        bool masked;
-
-        enum lex_format format; /* From the constant's lex_token. */
-    };
-
-    /* Null-terminated string constant. */
-    char *string;
-};
-
-/* A collection of "union expr_constant"s of the same type. */
-struct expr_constant_set {
-    union expr_constant *values;  /* Constants. */
-    size_t n_values;              /* Number of constants. */
-    enum expr_constant_type type; /* Type of the constants. */
-    bool in_curlies;              /* Whether the constants were in {}. */
-};
-
 /* A reference to a symbol or a subfield of a symbol.
  *
  * For string fields, ofs and n_bits are 0. */
@@ -456,7 +423,6 @@  struct expr_context {
 
 struct expr *expr_parse__(struct expr_context *);
 static void expr_not(struct expr *);
-static void expr_constant_set_destroy(struct expr_constant_set *);
 static bool parse_field(struct expr_context *, struct expr_field *);
 
 static bool
@@ -837,7 +803,7 @@  parse_constant_set(struct expr_context *ctx, struct expr_constant_set *cs)
     return ok;
 }
 
-static void
+void
 expr_constant_set_destroy(struct expr_constant_set *cs)
 {
     if (cs) {
@@ -2928,3 +2894,42 @@  exit:
     }
     return ctx.error;
 }
+
+bool
+expr_parse_constant_set(struct lexer *lexer, const struct shash *symtab,
+                        struct expr_constant_set *cs)
+{
+    struct expr_context ctx = {
+       .lexer = lexer,
+       .symtab = symtab,
+    };
+
+    return parse_constant_set(&ctx, cs);
+}
+
+struct expr_symbol_dhcp_opts *
+dhcp_opt_expr_symtab_add_field(struct shash *symtab, const char *opt_name,
+                               uint8_t opt_code, enum  dhcp_opt_type type)
+{
+    struct expr_symbol_dhcp_opts *symbol = xzalloc(sizeof *symbol);
+    symbol->opt_name = xstrdup(opt_name);
+    symbol->opt_code = opt_code;
+    symbol->opt_type = type;
+    shash_add_assert(symtab, symbol->opt_name, symbol);
+
+    return symbol;
+}
+
+void
+dhcp_opt_expr_symtab_destroy(struct shash *symtab)
+{
+    struct shash_node *node, *next;
+
+    SHASH_FOR_EACH_SAFE (node, next, symtab) {
+        struct expr_symbol_dhcp_opts *symbol = node->data;
+
+        shash_delete(symtab, node);
+        free(symbol->opt_name);
+        free(symbol);
+    }
+}
diff --git a/ovn/lib/expr.h b/ovn/lib/expr.h
index fa7ccbe..370c614 100644
--- a/ovn/lib/expr.h
+++ b/ovn/lib/expr.h
@@ -391,4 +391,61 @@  char *expr_parse_field(struct lexer *, int n_bits, bool rw,
                        const struct shash *symtab, struct mf_subfield *,
                        struct expr **prereqsp);
 
+/* Type of a "union expr_constant" or "struct expr_constant_set". */
+enum expr_constant_type {
+    EXPR_C_INTEGER,
+    EXPR_C_STRING
+};
+
+/* A string or integer constant (one must know which from context). */
+union expr_constant {
+    /* Integer constant.
+     *
+     * The width of a constant isn't always clear, e.g. if you write "1",
+     * there's no way to tell whether you mean for that to be a 1-bit constant
+     * or a 128-bit constant or somewhere in between. */
+    struct {
+        union mf_subvalue value;
+        union mf_subvalue mask; /* Only initialized if 'masked'. */
+        bool masked;
+
+        enum lex_format format; /* From the constant's lex_token. */
+    };
+
+    /* Null-terminated string constant. */
+    char *string;
+};
+
+/* A collection of "union expr_constant"s of the same type. */
+struct expr_constant_set {
+    union expr_constant *values;  /* Constants. */
+    size_t n_values;              /* Number of constants. */
+    enum expr_constant_type type; /* Type of the constants. */
+    bool in_curlies;              /* Whether the constants were in {}. */
+};
+
+bool expr_parse_constant_set(struct lexer *, const struct shash *symtab,
+                             struct expr_constant_set *cs);
+void expr_constant_set_destroy(struct expr_constant_set *cs);
+
+enum dhcp_opt_type {
+    DHCP_OPT_TYPE_BOOL,
+    DHCP_OPT_TYPE_UINT8,
+    DHCP_OPT_TYPE_UINT16,
+    DHCP_OPT_TYPE_UINT32,
+    DHCP_OPT_TYPE_IP4,
+    DHCP_OPT_TYPE_STR
+};
+
+struct expr_symbol_dhcp_opts {
+    char *opt_name;
+    uint8_t opt_code;
+    uint8_t opt_type;
+};
+
+struct expr_symbol_dhcp_opts *dhcp_opt_expr_symtab_add_field(
+    struct shash *symtab, const char *opt_name, uint8_t opt_code,
+    enum  dhcp_opt_type type);
+
+void dhcp_opt_expr_symtab_destroy(struct shash *symtab);
 #endif /* ovn/expr.h */
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4b1d611..0e3654a 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -89,10 +89,11 @@  enum ovn_stage {
     PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_L2,    0, "ls_in_port_sec_l2")     \
     PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_IP,    1, "ls_in_port_sec_ip")     \
     PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,    2, "ls_in_port_sec_nd")     \
-    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        3, "ls_in_pre_acl")      \
-    PIPELINE_STAGE(SWITCH, IN,  ACL,            4, "ls_in_acl")          \
-    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        5, "ls_in_arp_rsp")      \
-    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        6, "ls_in_l2_lkup")      \
+    PIPELINE_STAGE(SWITCH, IN,  DHCP,           3, "ls_in_dhcp")     \
+    PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        4, "ls_in_pre_acl")      \
+    PIPELINE_STAGE(SWITCH, IN,  ACL,            5, "ls_in_acl")          \
+    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        6, "ls_in_arp_rsp")      \
+    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        7, "ls_in_l2_lkup")      \
                                                                       \
     /* Logical switch egress stages. */                               \
     PIPELINE_STAGE(SWITCH, OUT, PRE_ACL,     0, "ls_out_pre_acl")     \
@@ -1324,6 +1325,61 @@  has_stateful_acl(struct ovn_datapath *od)
     return false;
 }
 
+static bool
+build_dhcp_action(struct ovn_port *op, ovs_be32 offer_ip, struct ds *action)
+{
+    uint8_t options_bmap = 0;
+    struct smap_node *node;
+    char *server_ip= NULL;
+    char *server_mac = NULL;
+
+    ds_put_format(action, "dhcp_offer(offerip = "IP_FMT", ",
+                  IP_ARGS(offer_ip));
+
+    SMAP_FOR_EACH(node, &op->od->nbs->dhcp_options) {
+        if(!strcmp(node->key, "router")) {
+            options_bmap |= 0x1;
+        } else if (!strcmp(node->key, "server_id")) {
+            options_bmap |= 0x2;
+            server_ip = node->value;
+        } else if (!strcmp(node->key, "server_mac")) {
+            options_bmap |= 0x4;
+            server_mac = node->value;
+            continue;
+        } else if (!strcmp(node->key, "lease_time")) {
+            options_bmap |= 0x8;
+        } else if (!strcmp(node->key, "netmask")) {
+            options_bmap |= 0x10;
+        }
+
+        ds_put_format(action, "%s = %s, ", node->key, node->value);
+    }
+
+    if (!(options_bmap & 0x08)) {
+        return false;
+    }
+
+    SMAP_FOR_EACH(node, &op->nbs->options) {
+        if(!strncmp(node->key, "dhcp-opt-", 9)) {
+            ds_put_format(action, "%s = %s, ", &node->key[9], node->value);
+        }
+    }
+
+    if (!(options_bmap & 0x10)) {
+        ds_put_cstr(action, "netmask = 255.255.255.0, ");
+    }
+
+    ds_chomp(action, ' ');
+    ds_chomp(action, ',');
+
+    ds_put_format(action, "); eth.dst = eth.src; eth.src = %s; ip4.dst = "IP_FMT"; "
+                              "ip4.src = %s; udp.src = 67; udp.dst = 68; ",
+                              server_mac, IP_ARGS(offer_ip), server_ip);
+    ds_put_cstr(action, "outport = inport; inport = \"\"; "
+                    "/* Allow sending out inport. */ output;");
+    return true;
+}
+
 static void
 build_acls(struct ovn_datapath *od, struct hmap *lflows, struct hmap *ports)
 {
@@ -1541,8 +1597,8 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         }
     }
 
-    /* Ingress table 1 and 2: Port security - IP and ND, by default goto next.
-     * (priority 0)*/
+    /* Ingress table 1 and 2: Port security - IP and ND, tabl3 - DCHP
+     * by default goto next. (priority 0)*/
     HMAP_FOR_EACH (od, key_node, datapaths) {
         if (!od->nbs) {
             continue;
@@ -1550,9 +1606,51 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
 
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 0, "1", "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_IP, 0, "1", "next;");
+        ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP, 0, "1", "next;");
     }
 
-    /* Ingress table 3: ARP responder, skip requests coming from localnet ports.
+    /* Logical switch ingress table 3: DHCP priority 50. */
+    HMAP_FOR_EACH (op, key_node, ports) {
+       if (!op->nbs) {
+           continue;
+       }
+
+       if (!lport_is_enabled(op->nbs)) {
+           /* Drop packets from disabled logical ports (since logical flow
+            * tables are default-drop). */
+           continue;
+       }
+
+       for (size_t i = 0; i < op->nbs->n_addresses; i++) {
+           struct lport_addresses laddrs;
+           if (!extract_lport_addresses(op->nbs->addresses[i], &laddrs,
+                                        false)) {
+               continue;
+           }
+
+           if (!laddrs.n_ipv4_addrs) {
+               continue;
+           }
+
+           struct ds action = DS_EMPTY_INITIALIZER;
+           if (build_dhcp_action(op, laddrs.ipv4_addrs[0].addr, &action)) {
+               struct ds match = DS_EMPTY_INITIALIZER;
+               ds_put_format(&match, "inport == %s && eth.src == "ETH_ADDR_FMT
+                             " && ip4.src == 0.0.0.0 && "
+                             "ip4.dst == 255.255.255.255 && udp.src == 68 && "
+                             "udp.dst == 67", op->json_key,
+                             ETH_ADDR_ARGS(laddrs.ea));
+
+               ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP, 50,
+                             ds_cstr(&match), ds_cstr(&action));
+               ds_destroy(&match);
+           }
+           ds_destroy(&action);
+       }
+
+    }
+
+    /* Ingress table 6: ARP responder, skip requests coming from localnet ports.
      * (priority 100). */
     HMAP_FOR_EACH (op, key_node, ports) {
         if (!op->nbs) {
@@ -1567,7 +1665,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         }
     }
 
-    /* Ingress table 5: ARP responder, reply for known IPs.
+    /* Ingress table 6: ARP responder, reply for known IPs.
      * (priority 50). */
     HMAP_FOR_EACH (op, key_node, ports) {
         if (!op->nbs) {
@@ -1617,7 +1715,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         }
     }
 
-    /* Ingress table 5: ARP responder, by default goto next.
+    /* Ingress table 6: ARP responder, by default goto next.
      * (priority 0)*/
     HMAP_FOR_EACH (od, key_node, datapaths) {
         if (!od->nbs) {
@@ -1627,7 +1725,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ARP_RSP, 0, "1", "next;");
     }
 
-    /* Ingress table 6: Destination lookup, broadcast and multicast handling
+    /* Ingress table 7: Destination lookup, broadcast and multicast handling
      * (priority 100). */
     HMAP_FOR_EACH (op, key_node, ports) {
         if (!op->nbs) {
@@ -1647,7 +1745,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                       "outport = \""MC_FLOOD"\"; output;");
     }
 
-    /* Ingress table 6: Destination lookup, unicast handling (priority 50), */
+    /* Ingress table 7: Destination lookup, unicast handling (priority 50), */
     HMAP_FOR_EACH (op, key_node, ports) {
         if (!op->nbs) {
             continue;
@@ -1684,7 +1782,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         }
     }
 
-    /* Ingress table 6: Destination lookup for unknown MACs (priority 0). */
+    /* Ingress table 7: Destination lookup for unknown MACs (priority 0). */
     HMAP_FOR_EACH (od, key_node, datapaths) {
         if (!od->nbs) {
             continue;
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 40a7a97..2088fd5 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
-    "version": "2.0.2",
-    "cksum": "4289495412 4436",
+    "version": "2.0.3",
+    "cksum": "776261543 4598",
     "tables": {
         "Logical_Switch": {
             "columns": {
@@ -16,6 +16,9 @@ 
                                           "refType": "strong"},
                                   "min": 0,
                                   "max": "unlimited"}},
+                "dhcp_options": {
+                     "type": {"key": "string", "value": "string",
+                              "min": 0, "max": "unlimited"}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index e65bc3a..803c213 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -78,6 +78,12 @@ 
         See <em>External IDs</em> at the beginning of this document.
       </column>
     </group>
+
+    <column name="dhcp_options">
+        This column provides key/value settings specific to the logical port
+        <ref column="type"/>.  The type-specific options are described
+        individually below.
+    </column>
   </table>
 
   <table name="Logical_Port" title="L2 logical switch port">
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bdad368..5bff15e 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -478,6 +478,52 @@  nbctl_lswitch_list(struct ctl_context *ctx)
     smap_destroy(&lswitches);
     free(nodes);
 }
+
+static void
+nbctl_lswitch_set_dhcp_options(struct ctl_context *ctx)
+{
+    const char *id = ctx->argv[1];
+    const struct nbrec_logical_switch *lswitch;
+    size_t i;
+    struct smap dhcp_options = SMAP_INITIALIZER(&dhcp_options);
+
+    lswitch = lswitch_by_name_or_uuid(ctx, id);
+    if (!lswitch) {
+        return;
+    }
+
+    for (i = 2; i < ctx->argc; i++) {
+        char *key, *value;
+        value = xstrdup(ctx->argv[i]);
+        key = strsep(&value, "=");
+        if (value) {
+            smap_add(&dhcp_options, key, value);
+        }
+        free(key);
+    }
+
+    nbrec_logical_switch_set_dhcp_options(lswitch, &dhcp_options);
+
+    smap_destroy(&dhcp_options);
+}
+
+static void
+nbctl_lswitch_get_dhcp_options(struct ctl_context *ctx)
+{
+    const char *id = ctx->argv[1];
+    const struct nbrec_logical_switch *lswitch;
+    struct smap_node *node;
+
+    lswitch = lswitch_by_name_or_uuid(ctx, id);
+    if (!lswitch) {
+        return;
+    }
+
+    SMAP_FOR_EACH(node, &lswitch->dhcp_options) {
+        ds_put_format(&ctx->output, "%s=%s\n", node->key, node->value);
+    }
+}
+
 
 static const struct nbrec_logical_port *
 lport_by_name_or_uuid(struct ctl_context *ctx, const char *id)
@@ -1311,6 +1357,11 @@  static const struct ctl_command_syntax nbctl_commands[] = {
     { "lswitch-del", 1, 1, "LSWITCH", NULL, nbctl_lswitch_del,
       NULL, "", RW },
     { "lswitch-list", 0, 0, "", NULL, nbctl_lswitch_list, NULL, "", RO },
+    { "lswitch-set-dhcp-options", 1, INT_MAX,
+      "LSWITCH KEY=VALUE [KEY=VALUE]...", NULL, nbctl_lswitch_set_dhcp_options,
+      NULL, "", RW },
+    { "lswitch-get-dhcp-options", 1, 1, "LSWITCH", NULL,
+      nbctl_lswitch_get_dhcp_options, NULL, "", RO },
 
     /* acl commands. */
     { "acl-add", 5, 5, "LSWITCH DIRECTION PRIORITY MATCH ACTION", NULL,