[ovs-dev] [ovs-dev,v2,5/8] ovn-controller: add 'nd_ra' action

Ben Pfaff blp at ovn.org
Tue Oct 4 17:18:59 UTC 2016


Hi Justin, I see that you reviewed patches 1-4 in this series.  Do you
plan to review the remaining patches too?  They are still in patchwork:
https://patchwork.ozlabs.org/project/openvswitch/list/?submitter=69084

On Sat, Sep 03, 2016 at 05:09:53AM +0000, Zong Kai LI wrote:
> This patchs add a new action 'nd_ra' against current 'nd_na' action, to work as
> a Router Solicitation packet responder. The inner actions to set fields, flags,
> options in RA packet will be introduced in the following patch.
> 
> v1 -> v2
> rebased
> ---
>  include/ovn/actions.h     |  7 ++++++
>  ovn/controller/pinctrl.c  | 61 +++++++++++++++++++++++++++++++----------------
>  ovn/lib/actions.c         | 28 ++++++++++++++++++++++
>  ovn/ovn-sb.xml            | 41 +++++++++++++++++++++++++++++++
>  ovn/utilities/ovn-trace.c | 19 ++++++++-------
>  tests/ovn.at              |  6 +++++
>  6 files changed, 133 insertions(+), 29 deletions(-)
> 
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index d1942b3..a776a26 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -61,6 +61,7 @@ struct simap;
>      OVNACT(CT_LB,         ovnact_ct_lb)             \
>      OVNACT(ARP,           ovnact_nest)              \
>      OVNACT(ND_NA,         ovnact_nest)              \
> +    OVNACT(ND_RA,         ovnact_nest)              \
>      OVNACT(GET_ARP,       ovnact_get_mac_bind)      \
>      OVNACT(PUT_ARP,       ovnact_put_mac_bind)      \
>      OVNACT(GET_ND,        ovnact_get_mac_bind)      \
> @@ -344,6 +345,12 @@ enum action_opcode {
>       *   - Any number of DHCPv6 options.
>       */
>      ACTION_OPCODE_PUT_DHCPV6_OPTS,
> +
> +    /* "nd_ra { ...actions... }".
> +     *
> +     * The actions, in OpenFlow 1.3 format, follow the action_header.
> +     */
> +    ACTION_OPCODE_ND_RA,
>  };
>  
>  /* Header. */
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 4ef2366..ae3a58b 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -71,9 +71,9 @@ static void send_garp_run(const struct ovsrec_bridge *,
>                            const char *chassis_id,
>                            const struct lport_index *lports,
>                            struct hmap *local_datapaths);
> -static void pinctrl_handle_nd_na(const struct flow *ip_flow,
> -                                 const struct match *md,
> -                                 struct ofpbuf *userdata);
> +static void pinctrl_handle_nd(const struct flow *ip_flow,
> +                              const struct match *md,
> +                              struct ofpbuf *userdata);
>  static void reload_metadata(struct ofpbuf *ofpacts,
>                              const struct match *md);
>  
> @@ -698,7 +698,8 @@ process_packet_in(const struct ofp_header *msg)
>          break;
>  
>      case ACTION_OPCODE_ND_NA:
> -        pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata);
> +    case ACTION_OPCODE_ND_RA:
> +        pinctrl_handle_nd(&headers, &pin.flow_metadata, &userdata);
>          break;
>  
>      case ACTION_OPCODE_PUT_ND:
> @@ -1350,33 +1351,51 @@ reload_metadata(struct ofpbuf *ofpacts, const struct match *md)
>  }
>  
>  static void
> -pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
> -                     struct ofpbuf *userdata)
> +pinctrl_handle_nd(const struct flow *ip_flow, const struct match *md,
> +                  struct ofpbuf *userdata)
>  {
> -    /* This action only works for IPv6 ND Neighbor Solicitation packets,
> -     * and the switch should only send us such packets this way, but check
> +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +    /* This action only works for IPv6 ND packets, and only Neighbor
> +     * Solicitation and Router Solicitation packets are supported.
> +     * The switch should only send us ND packets this way, but check
>       * here just to be sure. */
> -    if (!is_nd_neighbor(ip_flow, NULL)
> -        || ip_flow->tp_src != htons(ND_NEIGHBOR_SOLICIT)) {
> -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> -        VLOG_WARN_RL(&rl, "NA action on non-ND-NS packet");
> +    if (!is_nd(ip_flow, NULL)
> +        || (ip_flow->tp_src != htons(ND_NEIGHBOR_SOLICIT)
> +            && ip_flow->tp_src != htons(ND_ROUTER_SOLICIT))) {
> +        VLOG_WARN_RL(&rl, "ND action on invalid or unsupported packet");
>          return;
>      }
>  
>      enum ofp_version version = rconn_get_version(swconn);
>      enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
>  
> -    uint64_t packet_stub[128 / 8];
> +    bool as_nd_na = ip_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT);
> +    uint64_t packet_stub[256 / 8];
>      struct dp_packet packet;
>      dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
>  
> -    /* xxx These flags are not exactly correct.  Look at section 7.2.4
> -     * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
> -     * xxx router's interfaces and ND_RSO_SOLICITED only if it was
> -     * xxx requested. */
> -    compose_nd_na(&packet, ip_flow->dl_dst, ip_flow->dl_src,
> -                  &ip_flow->nd_target, &ip_flow->ipv6_src,
> -                  htonl(ND_RSO_SOLICITED | ND_RSO_OVERRIDE));
> +    if (as_nd_na) {
> +        /* xxx These flags are not exactly correct.  Look at section 7.2.4
> +         * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
> +         * xxx router's interfaces and ND_RSO_SOLICITED only if it was
> +         * xxx requested. */
> +        compose_nd_na(&packet, ip_flow->dl_dst, ip_flow->dl_src,
> +                      &ip_flow->nd_target, &ip_flow->ipv6_src,
> +                      htonl(ND_RSO_SOLICITED | ND_RSO_OVERRIDE));
> +    } else {
> +        /* xxx Using hardcode data to compose RA packet.
> +         * xxx The following patch should fix this. */
> +        uint8_t cur_hop_limit = 64;
> +        uint8_t mo_flags = 0;
> +        ovs_be16 router_lifetime = htons(0x2a30);  /* 3 hours. */
> +        ovs_be32 reachable_time = 0;
> +        ovs_be32 retrans_timer = 0;
> +
> +        compose_nd_ra(&packet, ip_flow->dl_dst, ip_flow->dl_src,
> +                      &ip_flow->ipv6_dst, &ip_flow->ipv6_src,
> +                      cur_hop_limit, mo_flags,
> +                      router_lifetime, reachable_time, retrans_timer);
> +    }
>  
>      /* Reload previous packet metadata. */
>      uint64_t ofpacts_stub[4096 / 8];
> @@ -1387,7 +1406,7 @@ pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
>                                                        version, &ofpacts);
>      if (error) {
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> -        VLOG_WARN_RL(&rl, "failed to parse actions for 'na' (%s)",
> +        VLOG_WARN_RL(&rl, "failed to parse actions for 'nd' (%s)",
>                       ofperr_to_string(error));
>          goto exit;
>      }
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 28b66ed..5568475 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -1066,6 +1066,12 @@ parse_ND_NA(struct action_context *ctx)
>  }
>  
>  static void
> +parse_ND_RA(struct action_context *ctx)
> +{
> +    parse_nested_action(ctx, OVNACT_ND_RA, "nd_rs");
> +}
> +
> +static void
>  format_nested_action(const struct ovnact_nest *on, const char *name,
>                       struct ds *s)
>  {
> @@ -1087,6 +1093,12 @@ format_ND_NA(const struct ovnact_nest *nest, struct ds *s)
>  }
>  
>  static void
> +format_ND_RA(const struct ovnact_nest *nest, struct ds *s)
> +{
> +    format_nested_action(nest, "nd_ra", s);
> +}
> +
> +static void
>  encode_nested_actions(const struct ovnact_nest *on,
>                        const struct ovnact_encode_params *ep,
>                        enum action_opcode opcode,
> @@ -1127,6 +1139,14 @@ encode_ND_NA(const struct ovnact_nest *on,
>  }
>  
>  static void
> +encode_ND_RA(const struct ovnact_nest *on,
> +             const struct ovnact_encode_params *ep,
> +             struct ofpbuf *ofpacts)
> +{
> +    encode_nested_actions(on, ep, ACTION_OPCODE_ND_RA, ofpacts);
> +}
> +
> +static void
>  free_nested_actions(struct ovnact_nest *on)
>  {
>      ovnacts_free(on->nested, on->nested_len);
> @@ -1144,6 +1164,12 @@ free_ND_NA(struct ovnact_nest *nest)
>  {
>      free_nested_actions(nest);
>  }
> +
> +static void
> +free_ND_RA(struct ovnact_nest *nest)
> +{
> +    free_nested_actions(nest);
> +}
>  
>  static void
>  parse_get_mac_bind(struct action_context *ctx, int width,
> @@ -1669,6 +1695,8 @@ parse_action(struct action_context *ctx)
>          parse_ARP(ctx);
>      } else if (lexer_match_id(ctx->lexer, "nd_na")) {
>          parse_ND_NA(ctx);
> +    } else if (lexer_match_id(ctx->lexer, "nd_ra")) {
> +        parse_ND_RA(ctx);
>      } else if (lexer_match_id(ctx->lexer, "get_arp")) {
>          parse_get_mac_bind(ctx, 32, ovnact_put_GET_ARP(ctx->ovnacts));
>      } else if (lexer_match_id(ctx->lexer, "put_arp")) {
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index af48daa..7b17d9d 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1210,6 +1210,47 @@
>            </p>
>          </dd>
>  
> +        <dt>
> +          <code>nd_ra { <var>action</var>; </code>...<code> };</code>
> +        </dt>
> +
> +        <dd>
> +          <p>
> +            Temporarily replaces the IPv6 router solicitation (RS) packet
> +            being processed by an IPv6 router advertisement (RA)
> +            packet and executes each nested <var>action</var> on the RA
> +            packet.  Actions following the <code>nd_ra</code> action,
> +            if any, apply to the original, unmodified packet.
> +          </p>
> +
> +          <p>
> +            The RA packet that this action operates on is initialized based on
> +            the IPv6 packet being processed, as follows. These are default
> +            values that the nested actions will probably want to change:
> +          </p>
> +
> +          <ul>
> +            <li><code>eth.dst</code> copied from <code>eth.src</code></li>
> +            <li><code>eth.src</code> replaced by new value</li>
> +            <li><code>eth.type = 0x86dd</code></li>
> +            <li><code>ip6.dst</code> copied from <code>ip6.src</code></li>
> +            <li><code>ip6.src</code> replaced by new value</li>
> +            <li><code>icmp6.type = 134</code> (Router Advertisement)</li>
> +            <li><code>nd.target</code> unchanged</li>
> +            <li><code>nd.tll = 00:00:00:00:00:00</code></li>
> +            <li><code>nd.sll</code> replaced by new value</li>
> +          </ul>
> +
> +          <p>
> +            The ND packet has the same VLAN header, if any, as the IPv6 packet
> +            it replaces.
> +          </p>
> +
> +          <p>
> +            <b>Prerequisite:</b> <code>nd_rs</code>
> +          </p>
> +        </dd>
> +
>          <dt><code>get_nd(<var>P</var>, <var>A</var>);</code></dt>
>  
>          <dd>
> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> index 487ae52..fabecc6 100644
> --- a/ovn/utilities/ovn-trace.c
> +++ b/ovn/utilities/ovn-trace.c
> @@ -1128,23 +1128,25 @@ execute_arp(const struct ovnact_nest *on, const struct ovntrace_datapath *dp,
>  }
>  
>  static void
> -execute_nd_na(const struct ovnact_nest *on, const struct ovntrace_datapath *dp,
> -              const struct flow *uflow, uint8_t table_id,
> -              enum ovntrace_pipeline pipeline, struct ovs_list *super)
> +execute_nd_adv(const struct ovnact_nest *on, const struct ovntrace_datapath *dp,
> +               const struct flow *uflow, uint8_t table_id,
> +               enum ovntrace_pipeline pipeline, struct ovs_list *super,
> +               bool is_neighbor_adv)
>  {
>      struct flow na_flow = *uflow;
>  
> -    /* Update fields for NA. */
> +    /* Update fields for ND Router/Neighbor Advertisement. */
>      na_flow.dl_src = uflow->dl_dst;
>      na_flow.dl_dst = uflow->dl_src;
>      na_flow.ipv6_dst = uflow->ipv6_src;
>      na_flow.ipv6_src = uflow->nd_target;
> -    na_flow.tp_src = htons(136);
> +    na_flow.tp_src = is_neighbor_adv ? htons(136) : htons(134);
>      na_flow.arp_sha = eth_addr_zero;
>      na_flow.arp_tha = uflow->dl_dst;
>  
>      struct ovntrace_node *node = ovntrace_node_append(
> -        super, OVNTRACE_NODE_TRANSFORMATION, "nd_na");
> +        super, OVNTRACE_NODE_TRANSFORMATION,
> +        (is_neighbor_adv ? "nd_na" : "nd_ra"));
>  
>      trace_actions(on->nested, on->nested_len, dp, &na_flow,
>                    table_id, pipeline, &node->subs);
> @@ -1264,8 +1266,9 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
>              break;
>  
>          case OVNACT_ND_NA:
> -            execute_nd_na(ovnact_get_ND_NA(a), dp, uflow, table_id, pipeline,
> -                          super);
> +	case OVNACT_ND_RA:
> +            execute_nd_adv(ovnact_get_ND_NA(a), dp, uflow, table_id, pipeline,
> +                           super, (a->type == OVNACT_ND_NA));
>              break;
>  
>          case OVNACT_GET_ARP:
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 77ce244..ec16d8e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -885,6 +885,12 @@ nd_na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport = inpor
>      encodes as controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
>      has prereqs nd_ns
>  
> +# nd_ra
> +nd_ra{eth.src = 12:34:56:78:9a:bc; ip6.src = fdad:1234:5678::1; outport = inport; flags.loopback = 1; output;};
> +    formats as nd_ra { eth.src = 12:34:56:78:9a:bc; ip6.src = fdad:1234:5678::1; outport = inport; flags.loopback = 1; output; };
> +    encodes as controller(userdata=00.00.00.06.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.18.80.00.34.10.fd.ad.12.34.56.78.00.00.00.00.00.00.00.00.00.01.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.ff.ff.00.18.00.00.23.20.00.07.00.00.00.01.14.04.00.00.00.00.00.00.00.01.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> +    has prereqs nd_rs
> +
>  # get_nd
>  get_nd(outport, ip6.dst);
>      encodes as push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_XXREG0[]



More information about the dev mailing list