This looks pretty much on the right track, just minor comments.

I think setting the tos bits is sufficiently unrelated, that it
belongs in it's own patch. Would you mind separating it out?

Please add a short description of the check_tnl_key configuration
option to the bfd section of vswitch.xml which is used to generate the
man pages.  the CFM version of the feature is already documented, so I
think you can probably just copy that.

Ethan

On Mon, Jul 15, 2013 at 10:29 AM, Pavithra Ramesh <param...@vmware.com> wrote:
> This change adds the check_tnl_key functionality for BFD.
> It also populates the ToS field for BFD packets.
>
> Signed-off-by: Pavithra Ramesh <param...@vmware.com>
> ---
>  lib/bfd.c                    |   19 ++++++++++++-------
>  lib/bfd.h                    |    4 +++-
>  ofproto/ofproto-dpif-xlate.c |    2 +-
>  3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index aa1a3f7..e537915 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -56,11 +56,7 @@ VLOG_DEFINE_THIS_MODULE(bfd);
>   *
>   * - BFD show into ovs-bugtool.
>   *
> - * - Set TOS/PCP on inner BFD frame, and outer tunnel header when encapped.
> - *
> - * - CFM "check_tnl_key" option equivalent.
> - *
> - * - CFM "fault override" equivalent.
> + * - Set TOS/PCP on outer tunnel header when encapped.
>   *
>   * - Sending BFD messages should be in its own thread/process.
>   *
> @@ -182,6 +178,7 @@ struct bfd {
>
>      int ref_cnt;
>      int forwarding_override;      /* Manual override of 'forwarding' status. 
> */
> +    bool check_tnl_key;    /* Verify the tunnel key of inbound packets? */
>  };
>
>  static bool bfd_in_poll(const struct bfd *);
> @@ -287,6 +284,7 @@ bfd_configure(struct bfd *bfd, const char *name,
>          bfd_set_state(bfd, STATE_DOWN, DIAG_NONE);
>      }
>
> +    bfd->check_tnl_key = smap_get_bool(cfg, "check_tnl_key", false);
>      min_tx = smap_get_int(cfg, "min_tx", 100);
>      min_tx = MAX(min_tx, 100);
>      if (bfd->cfg_min_tx != min_tx) {
> @@ -409,6 +407,7 @@ bfd_put_packet(struct bfd *bfd, struct ofpbuf *p,
>      ip->ip_ihl_ver = IP_IHL_VER(5, 4);
>      ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
>      ip->ip_ttl = 255;
> +    ip->ip_tos = 0x1110; /* Minimize delay, maximize throughput, reliability 
> */
>      ip->ip_proto = IPPROTO_UDP;
>      ip->ip_src = htonl(0xA9FE0100); /* 169.254.1.0 Link Local. */
>      ip->ip_dst = htonl(0xA9FE0101); /* 169.254.1.1 Link Local. */
> @@ -449,13 +448,19 @@ bfd_put_packet(struct bfd *bfd, struct ofpbuf *p,
>  }
>
>  bool
> -bfd_should_process_flow(const struct flow *flow, struct flow_wildcards *wc)
> +bfd_should_process_flow(const struct bfd *bfd,
> +                        const struct flow *flow,
> +                        struct flow_wildcards *wc)
>  {
>      memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>      memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
> +    if (bfd->check_tnl_key) {
> +        memset(&wc->masks.tunnel.tun_id, 0xff, sizeof 
> wc->masks.tunnel.tun_id);
> +    }
>      return (flow->dl_type == htons(ETH_TYPE_IP)
>              && flow->nw_proto == IPPROTO_UDP
> -            && flow->tp_dst == htons(3784));
> +            && flow->tp_dst == htons(3784)
> +            && (!bfd->check_tnl_key || flow->tunnel.tun_id == htonl(0)));
>  }
>
>  void
> diff --git a/lib/bfd.h b/lib/bfd.h
> index ab854d8..dc720a9 100644
> --- a/lib/bfd.h
> +++ b/lib/bfd.h
> @@ -34,7 +34,9 @@ bool bfd_should_send_packet(const struct bfd *);
>  void bfd_put_packet(struct bfd *bfd, struct ofpbuf *packet,
>                      uint8_t eth_src[6]);
>
> -bool bfd_should_process_flow(const struct flow *, struct flow_wildcards *);
> +bool bfd_should_process_flow(const struct bfd *,
> +                             const struct flow *,
> +                             struct flow_wildcards *);
>  void bfd_process_packet(struct bfd *, const struct flow *,
>                          const struct ofpbuf *);
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 84677a1..0507f8e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1193,7 +1193,7 @@ process_special(struct xlate_ctx *ctx, const struct 
> flow *flow,
>              cfm_process_heartbeat(xport->cfm, packet);
>          }
>          return SLOW_CFM;
> -    } else if (xport->bfd && bfd_should_process_flow(flow, wc)) {
> +    } else if (xport->bfd && bfd_should_process_flow(xport->bfd, flow, wc)) {
>          if (packet) {
>              bfd_process_packet(xport->bfd, flow, packet);
>          }
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
X-CudaMail-Whitelist-To: dev@openvswitch.org
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to