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

[Russell Bryant]

On Mon, Apr 4, 2016 at 8:46 AM, Ryan Moats <rmoats at us.ibm.com> wrote:

>
> "dev" <dev-bounces at openvswitch.org> wrote on 04/04/2016 06:41:01 AM:
>
> > From: Numan Siddique <nusiddiq at redhat.com>
> > To: ovs dev <dev at 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 at 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 at 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.

-- 
Russell Bryant