Re: [ovs-dev] [PATCH v3 2/2] OVN: Add support for periodic router advertisements.

2017-11-12 Thread Mark Michelson
On Fri, Nov 10, 2017 at 10:19 AM Jakub Sitnicki  wrote:

> Hi Mark,
>
> A couple more things caught my attention. Otherwise than that it LGTM.
>
> On Fri, Nov 03, 2017 at 09:24 PM GMT, Mark Michelson wrote:
> > This change adds three new options to the Northbound
> > Logical_Router_Port's ipv6_ra_configs option:
> >
> > * send_periodic: If set to "true", then OVN will send periodic router
> > advertisements out of this router port.
> > * max_interval: The maximum amount of time to wait between sending
> > periodic router advertisements.
> > * min_interval: The minimum amount of time to wait between sending
> > periodic router advertisements.
> >
> > When send_periodic is true, then IPv6 RA configs, as well as some layer
> > 2 and layer 3 information about the router port, are copied to the
> > southbound database. From there, ovn-controller can use this information
> > to know when to send periodic RAs and what to send in them.
> >
> > Because periodic RAs originate from each ovn-controller, the new
> > keep-local flag is set on the packet so that ports don't receive an
> > overabundance of RAs.
> >
> > Signed-off-by: Mark Michelson 
> > ---
> >  lib/packets.h|   7 +
> >  ovn/controller/pinctrl.c | 341
> +++
> >  ovn/northd/ovn-northd.c  |  65 +
> >  ovn/ovn-nb.xml   |  19 +++
> >  tests/ovn-northd.at  | 110 +++
> >  tests/ovn.at | 150 +
> >  6 files changed, 692 insertions(+)
> >
> > diff --git a/lib/packets.h b/lib/packets.h
> > index 057935cbf..b8249047f 100644
> > --- a/lib/packets.h
> > +++ b/lib/packets.h
> > @@ -976,6 +976,7 @@ BUILD_ASSERT_DECL(ND_PREFIX_OPT_LEN == sizeof(struct
> ovs_nd_prefix_opt));
> >
> >  /* Neighbor Discovery option: MTU. */
> >  #define ND_MTU_OPT_LEN 8
> > +#define ND_MTU_DEFAULT 0
> >  struct ovs_nd_mtu_opt {
> >  uint8_t  type;  /* ND_OPT_MTU */
> >  uint8_t  len;   /* Always 1. */
> > @@ -1015,6 +1016,12 @@ BUILD_ASSERT_DECL(RA_MSG_LEN == sizeof(struct
> ovs_ra_msg));
> >  #define ND_RA_MANAGED_ADDRESS 0x80
> >  #define ND_RA_OTHER_CONFIG0x40
> >
> > +/* Defaults based on MaxRtrInterval and MinRtrInterval from RFC 4861
> section
> > + * 6.2.1
> > + */
> > +#define ND_RA_MAX_INTERVAL_DEFAULT 600
> > +#define ND_RA_MIN_INTERVAL_DEFAULT(max) ((max) >= 9 ? (max) / 3 : (max)
> * 3 / 4)
> > +
> >  /*
> >   * Use the same struct for MLD and MLD2, naming members as the defined
> fields in
> >   * in the corresponding version of the protocol, though they are
> reserved in the
> > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> > index 29b2dde0c..428d5048f 100644
> > --- a/ovn/controller/pinctrl.c
> > +++ b/ovn/controller/pinctrl.c
> > @@ -48,6 +48,7 @@
> >  #include "socket-util.h"
> >  #include "timeval.h"
> >  #include "vswitch-idl.h"
> > +#include "lflow.h"
> >
> >  VLOG_DEFINE_THIS_MODULE(pinctrl);
> >
> > @@ -88,6 +89,11 @@ static void pinctrl_handle_put_nd_ra_opts(
> >  static void pinctrl_handle_nd_ns(const struct flow *ip_flow,
> >   const struct match *md,
> >   struct ofpbuf *userdata);
> > +static void init_ipv6_ras(void);
> > +static void destroy_ipv6_ras(void);
> > +static void ipv6_ra_wait(void);
> > +static void send_ipv6_ras(const struct controller_ctx *ctx,
> > +struct hmap *local_datapaths);
> >
> >  COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
> >
> > @@ -98,6 +104,7 @@ pinctrl_init(void)
> >  conn_seq_no = 0;
> >  init_put_mac_bindings();
> >  init_send_garps();
> > +init_ipv6_ras();
> >  }
> >
> >  static ovs_be32
> > @@ -1083,8 +1090,340 @@ pinctrl_run(struct controller_ctx *ctx,
> >  run_put_mac_bindings(ctx);
> >  send_garp_run(ctx, br_int, chassis, chassis_index, local_datapaths,
> >active_tunnels);
> > +send_ipv6_ras(ctx, local_datapaths);
> >  }
> >
> > +/* Table of ipv6_ra_state structures, keyed on logical port name */
> > +static struct shash ipv6_ras;
> > +
> > +/* Next IPV6 RA in seconds. */
> > +static long long int send_ipv6_ra_time;
> > +
> > +struct ipv6_network_prefix {
> > +struct in6_addr addr;
> > +unsigned int prefix_len;
> > +};
> > +
> > +struct ipv6_ra_config {
> > +time_t min_interval;
> > +time_t max_interval;
> > +struct eth_addr eth_src;
> > +struct eth_addr eth_dst;
> > +struct in6_addr ipv6_src;
> > +struct in6_addr ipv6_dst;
> > +ovs_be32 mtu;
> > +uint8_t mo_flags; /* Managed/Other flags for RAs */
> > +uint8_t la_flags; /* On-link/autonomous flags for address prefixes
> */
> > +struct ipv6_network_prefix *prefixes;
> > +int n_prefixes;
> > +};
> > +
> > +struct ipv6_ra_state {
> > +long long int next_announce;
> > +struct ipv6_ra_config *config;
> > +int64_t port_key;
> > +int64_t metadata;
> > +bool deleteme;
> > +};
> > +
> > +static void
> > 

Re: [ovs-dev] [PATCH v3 2/2] OVN: Add support for periodic router advertisements.

2017-11-10 Thread Jakub Sitnicki
Hi Mark,

A couple more things caught my attention. Otherwise than that it LGTM.

On Fri, Nov 03, 2017 at 09:24 PM GMT, Mark Michelson wrote:
> This change adds three new options to the Northbound
> Logical_Router_Port's ipv6_ra_configs option:
>
> * send_periodic: If set to "true", then OVN will send periodic router
> advertisements out of this router port.
> * max_interval: The maximum amount of time to wait between sending
> periodic router advertisements.
> * min_interval: The minimum amount of time to wait between sending
> periodic router advertisements.
>
> When send_periodic is true, then IPv6 RA configs, as well as some layer
> 2 and layer 3 information about the router port, are copied to the
> southbound database. From there, ovn-controller can use this information
> to know when to send periodic RAs and what to send in them.
>
> Because periodic RAs originate from each ovn-controller, the new
> keep-local flag is set on the packet so that ports don't receive an
> overabundance of RAs.
>
> Signed-off-by: Mark Michelson 
> ---
>  lib/packets.h|   7 +
>  ovn/controller/pinctrl.c | 341 
> +++
>  ovn/northd/ovn-northd.c  |  65 +
>  ovn/ovn-nb.xml   |  19 +++
>  tests/ovn-northd.at  | 110 +++
>  tests/ovn.at | 150 +
>  6 files changed, 692 insertions(+)
>
> diff --git a/lib/packets.h b/lib/packets.h
> index 057935cbf..b8249047f 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -976,6 +976,7 @@ BUILD_ASSERT_DECL(ND_PREFIX_OPT_LEN == sizeof(struct 
> ovs_nd_prefix_opt));
>
>  /* Neighbor Discovery option: MTU. */
>  #define ND_MTU_OPT_LEN 8
> +#define ND_MTU_DEFAULT 0
>  struct ovs_nd_mtu_opt {
>  uint8_t  type;  /* ND_OPT_MTU */
>  uint8_t  len;   /* Always 1. */
> @@ -1015,6 +1016,12 @@ BUILD_ASSERT_DECL(RA_MSG_LEN == sizeof(struct 
> ovs_ra_msg));
>  #define ND_RA_MANAGED_ADDRESS 0x80
>  #define ND_RA_OTHER_CONFIG0x40
>
> +/* Defaults based on MaxRtrInterval and MinRtrInterval from RFC 4861 section
> + * 6.2.1
> + */
> +#define ND_RA_MAX_INTERVAL_DEFAULT 600
> +#define ND_RA_MIN_INTERVAL_DEFAULT(max) ((max) >= 9 ? (max) / 3 : (max) * 3 
> / 4)
> +
>  /*
>   * Use the same struct for MLD and MLD2, naming members as the defined 
> fields in
>   * in the corresponding version of the protocol, though they are reserved in 
> the
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 29b2dde0c..428d5048f 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -48,6 +48,7 @@
>  #include "socket-util.h"
>  #include "timeval.h"
>  #include "vswitch-idl.h"
> +#include "lflow.h"
>
>  VLOG_DEFINE_THIS_MODULE(pinctrl);
>
> @@ -88,6 +89,11 @@ static void pinctrl_handle_put_nd_ra_opts(
>  static void pinctrl_handle_nd_ns(const struct flow *ip_flow,
>   const struct match *md,
>   struct ofpbuf *userdata);
> +static void init_ipv6_ras(void);
> +static void destroy_ipv6_ras(void);
> +static void ipv6_ra_wait(void);
> +static void send_ipv6_ras(const struct controller_ctx *ctx,
> +struct hmap *local_datapaths);
>
>  COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
>
> @@ -98,6 +104,7 @@ pinctrl_init(void)
>  conn_seq_no = 0;
>  init_put_mac_bindings();
>  init_send_garps();
> +init_ipv6_ras();
>  }
>
>  static ovs_be32
> @@ -1083,8 +1090,340 @@ pinctrl_run(struct controller_ctx *ctx,
>  run_put_mac_bindings(ctx);
>  send_garp_run(ctx, br_int, chassis, chassis_index, local_datapaths,
>active_tunnels);
> +send_ipv6_ras(ctx, local_datapaths);
>  }
>
> +/* Table of ipv6_ra_state structures, keyed on logical port name */
> +static struct shash ipv6_ras;
> +
> +/* Next IPV6 RA in seconds. */
> +static long long int send_ipv6_ra_time;
> +
> +struct ipv6_network_prefix {
> +struct in6_addr addr;
> +unsigned int prefix_len;
> +};
> +
> +struct ipv6_ra_config {
> +time_t min_interval;
> +time_t max_interval;
> +struct eth_addr eth_src;
> +struct eth_addr eth_dst;
> +struct in6_addr ipv6_src;
> +struct in6_addr ipv6_dst;
> +ovs_be32 mtu;
> +uint8_t mo_flags; /* Managed/Other flags for RAs */
> +uint8_t la_flags; /* On-link/autonomous flags for address prefixes */
> +struct ipv6_network_prefix *prefixes;
> +int n_prefixes;
> +};
> +
> +struct ipv6_ra_state {
> +long long int next_announce;
> +struct ipv6_ra_config *config;
> +int64_t port_key;
> +int64_t metadata;
> +bool deleteme;
> +};
> +
> +static void
> +init_ipv6_ras(void)
> +{
> +shash_init(_ras);
> +send_ipv6_ra_time = LLONG_MAX;
> +}
> +
> +static void
> +ipv6_ra_config_delete(struct ipv6_ra_config *config)
> +{
> +if (config) {
> +free(config->prefixes);
> +}
> +free(config);
> +}
> +
> +static void
> +ipv6_ra_delete(struct ipv6_ra_state *ra)
> +{
> +