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

[Ben Pfaff]

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 at 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