diff mbox series

[ovs-dev,PATCHv2] bfd: Add IPv6 address support to BFD packets

Message ID 20220802200313.261233-1-cpp.code.lv@gmail.com
State Changes Requested
Headers show
Series [ovs-dev,PATCHv2] bfd: Add IPv6 address support to BFD packets | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Toms Atteka Aug. 2, 2022, 8:03 p.m. UTC
Added config options for source and destination IPv6 addresses to
BFD packets. IPv6 address presence overrides IPv4 address and
packets are crafted as IPv6.

Added IP address values to status to ease setting up IP addresses.

Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
---
 lib/bfd.c            | 119 ++++++++++++++++++++++++++++++++++++++-----
 tests/bfd.at         |  30 +++++++++++
 vswitchd/vswitch.xml |  10 ++++
 3 files changed, 147 insertions(+), 12 deletions(-)

Comments

Toms Atteka Sept. 15, 2022, 7:17 p.m. UTC | #1
Been hangin here for a while, would be happy for a review.

On Tue, Aug 2, 2022 at 1:04 PM Toms Atteka <cpp.code.lv@gmail.com> wrote:
>
> Added config options for source and destination IPv6 addresses to
> BFD packets. IPv6 address presence overrides IPv4 address and
> packets are crafted as IPv6.
>
> Added IP address values to status to ease setting up IP addresses.
>
> Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
> ---
>  lib/bfd.c            | 119 ++++++++++++++++++++++++++++++++++++++-----
>  tests/bfd.at         |  30 +++++++++++
>  vswitchd/vswitch.xml |  10 ++++
>  3 files changed, 147 insertions(+), 12 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index 9698576d0..f9f359e99 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -187,6 +187,9 @@ struct bfd {
>      ovs_be32 ip_src;              /* IPv4 source address. */
>      ovs_be32 ip_dst;              /* IPv4 destination address. */
>
> +    struct in6_addr ipv6_src;              /* IPv6 source address. */
> +    struct in6_addr ipv6_dst;              /* IPv6 destination address. */
> +
>      uint16_t udp_src;             /* UDP source port. */
>
>      /* All timers in milliseconds. */
> @@ -244,10 +247,14 @@ static struct hmap *const all_bfds OVS_GUARDED_BY(mutex) = &all_bfds__;
>
>  static void bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *ip)
>      OVS_REQUIRES(mutex);
> +static void bfd_lookup_ipv6(const char *host_name, struct in6_addr *ipv6)
> +    OVS_REQUIRES(mutex);
>  static bool bfd_forwarding__(struct bfd *) OVS_REQUIRES(mutex);
>  static bool bfd_in_poll(const struct bfd *) OVS_REQUIRES(mutex);
>  static void bfd_poll(struct bfd *bfd) OVS_REQUIRES(mutex);
>  static const char *bfd_diag_str(enum diag) OVS_REQUIRES(mutex);
> +static const char *bfd_ip_str(ovs_be32 ip) OVS_REQUIRES(mutex);
> +static const char *bfd_ipv6_str(struct in6_addr ipv6) OVS_REQUIRES(mutex);
>  static const char *bfd_state_str(enum state) OVS_REQUIRES(mutex);
>  static long long int bfd_min_tx(const struct bfd *) OVS_REQUIRES(mutex);
>  static long long int bfd_tx_interval(const struct bfd *)
> @@ -332,6 +339,18 @@ bfd_get_status(const struct bfd *bfd, struct smap *smap)
>      smap_add_format(smap, "flap_count", "%"PRIu64, bfd->flap_count);
>      smap_add(smap, "remote_state", bfd_state_str(bfd->rmt_state));
>      smap_add(smap, "remote_diagnostic", bfd_diag_str(bfd->rmt_diag));
> +    if (bfd->ip_src) {
> +        smap_add(smap, "bfd_src_ip", bfd_ip_str(bfd->ip_src));
> +    }
> +    if (bfd->ip_dst) {
> +        smap_add(smap, "bfd_dst_ip", bfd_ip_str(bfd->ip_dst));
> +    }
> +    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
> +       smap_add(smap, "bfd_src_ipv6", bfd_ipv6_str(bfd->ipv6_src));
> +    }
> +    if (ipv6_addr_is_set(&bfd->ipv6_dst)) {
> +        smap_add(smap, "bfd_dst_ipv6", bfd_ipv6_str(bfd->ipv6_dst));
> +    }
>      ovs_mutex_unlock(&mutex);
>  }
>
> @@ -465,6 +484,10 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
>      bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
>                    htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst);
>
> +    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_src_ipv6", ""), &bfd->ipv6_src);
> +    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_dst_ipv6", ""), &bfd->ipv6_dst);
> +
> +
>      forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
>      if (bfd->forwarding_if_rx != forwarding_if_rx) {
>          bfd->forwarding_if_rx = forwarding_if_rx;
> @@ -588,6 +611,7 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
>      struct udp_header *udp;
>      struct eth_header *eth;
>      struct ip_header *ip;
> +    struct ovs_16aligned_ip6_hdr *ip6;
>      struct msg *msg;
>
>      ovs_mutex_lock(&mutex);
> @@ -611,23 +635,40 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
>          ? eth_src : bfd->local_eth_src;
>      eth->eth_dst = eth_addr_is_zero(bfd->local_eth_dst)
>          ? eth_addr_bfd : bfd->local_eth_dst;
> -    eth->eth_type = htons(ETH_TYPE_IP);
> -
> -    ip = dp_packet_put_zeros(p, sizeof *ip);
> -    ip->ip_ihl_ver = IP_IHL_VER(5, 4);
> -    ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
> -    ip->ip_ttl = MAXTTL;
> -    ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
> -    ip->ip_proto = IPPROTO_UDP;
> -    put_16aligned_be32(&ip->ip_src, bfd->ip_src);
> -    put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
> -    /* Checksum has already been zeroed by put_zeros call. */
> -    ip->ip_csum = csum(ip, sizeof *ip);
> +
> +    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
> +        eth->eth_type = htons(ETH_TYPE_IPV6);
> +
> +        ip6 = dp_packet_put_zeros(p, sizeof *ip6);
> +
> +        ip6->ip6_vfc = 0x60;
> +        ip6->ip6_hlim = MAXTTL;
> +        ip6->ip6_nxt = IPPROTO_UDP;
> +        ip6->ip6_plen = htons(sizeof *udp + sizeof *msg);
> +
> +        memcpy(&ip6->ip6_src, &bfd->ipv6_src, sizeof(bfd->ipv6_src));
> +        memcpy(&ip6->ip6_dst, &bfd->ipv6_dst, sizeof(bfd->ipv6_dst));
> +
> +    } else {
> +        eth->eth_type = htons(ETH_TYPE_IP);
> +
> +        ip = dp_packet_put_zeros(p, sizeof *ip);
> +        ip->ip_ihl_ver = IP_IHL_VER(5, 4);
> +        ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
> +        ip->ip_ttl = MAXTTL;
> +        ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
> +        ip->ip_proto = IPPROTO_UDP;
> +        put_16aligned_be32(&ip->ip_src, bfd->ip_src);
> +        put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
> +        /* Checksum has already been zeroed by put_zeros call. */
> +        ip->ip_csum = csum(ip, sizeof *ip);
> +    }
>
>      udp = dp_packet_put_zeros(p, sizeof *udp);
>      udp->udp_src = htons(bfd->udp_src);
>      udp->udp_dst = htons(BFD_DEST_PORT);
>      udp->udp_len = htons(sizeof *udp + sizeof *msg);
> +    udp->udp_csum = htons(0xffff);
>
>      msg = dp_packet_put_uninit(p, sizeof *msg);
>      msg->vers_diag = (BFD_VERSION << 5) | bfd->diag;
> @@ -674,6 +715,27 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow,
>          }
>      }
>
> +    if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> +        memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> +        if (flow->nw_proto == IPPROTO_UDP
> +            && !(flow->nw_frag & FLOW_NW_FRAG_LATER)
> +            && tp_dst_equals(flow, BFD_DEST_PORT, wc)
> +            && ipv6_addr_equals(&bfd->ipv6_src, &flow->ipv6_dst)) {
> +
> +            memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
> +
> +            bool check_tnl_key;
> +
> +            atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key);
> +            if (check_tnl_key) {
> +                memset(&wc->masks.tunnel.tun_id, 0xff,
> +                       sizeof wc->masks.tunnel.tun_id);
> +                return flow->tunnel.tun_id == htonll(0);
> +            }
> +            return true;
> +        }
> +    }
> +
>      if (flow->dl_type == htons(ETH_TYPE_IP)) {
>          memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>          if (flow->nw_proto == IPPROTO_UDP
> @@ -958,6 +1020,17 @@ bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *addr)
>      *addr = def;
>  }
>
> +static void
> +bfd_lookup_ipv6(const char *host_name, struct in6_addr *addr)
> +{
> +    if (host_name[0]) {
> +        if (ipv6_parse(host_name, addr)) {
> +            return;
> +        }
> +        VLOG_ERR_RL(&rl, "\"%s\" is not a valid IPv6 address", host_name);
> +    }
> +}
> +
>  static bool
>  bfd_in_poll(const struct bfd *bfd) OVS_REQUIRES(mutex)
>  {
> @@ -1083,6 +1156,28 @@ bfd_diag_str(enum diag diag) {
>      }
>  };
>
> +static const char *bfd_ip_str(ovs_be32 ip) {
> +    static char buf[INET_ADDRSTRLEN];
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +    ds_put_format(&ds, IP_FMT, IP_ARGS(ip));
> +
> +    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
> +    ds_destroy(&ds);
> +    return buf;
> +}
> +
> +static const char *bfd_ipv6_str(struct in6_addr ipv6) {
> +    static char buf[INET6_ADDRSTRLEN];
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +    ipv6_format_addr(&ipv6, &ds);
> +
> +    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
> +    ds_destroy(&ds);
> +    return buf;
> +}
> +
>  static void
>  log_msg(enum vlog_level level, const struct msg *p, const char *message,
>          const struct bfd *bfd) OVS_REQUIRES(mutex)
> diff --git a/tests/bfd.at b/tests/bfd.at
> index f5c6409f6..14aac0a19 100644
> --- a/tests/bfd.at
> +++ b/tests/bfd.at
> @@ -1133,3 +1133,33 @@ Datapath actions: 100
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +AT_SETUP([bfd - ipv6])
> +OVS_VSWITCHD_START(
> +  [add-br br1 -- \
> +   set bridge br1 datapath-type=dummy -- \
> +])
> +
> +AT_CHECK([ovs-vsctl add-port br0 p1 -- \
> +   set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::1 bfd:bfd_dst_ipv6=fe80::2 -- \
> +])
> +
> +AT_CHECK([ovs-vsctl add-port br1 p2 -- \
> +   set interface p2 type=dummy options:stream=unix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::2 bfd:bfd_dst_ipv6=fe80::1 -- \
> +])
> +
> +ovs-appctl time/stop
> +ovs-appctl time/warp 4100 100
> +
> +# Forwarding should be true
> +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
> +BFD_CHECK([p2], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
> +
> +# Disable bfd on p2, forwarding on p1 should go to false
> +AT_CHECK([ovs-vsctl set interface p2 bfd:enable=false])
> +
> +ovs-appctl time/warp 5000 100
> +BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index cc1dd77ec..e4a8d8d6d 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3964,6 +3964,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>            for transmitted BFD packets.  The default is <code>169.254.1.0</code>.
>          </column>
>
> +        <column name="bfd" key="bfd_src_ipv6">
> +          Set to an IPv6 address to set the IP address used as source for
> +          transmitted BFD packets.
> +        </column>
> +
> +        <column name="bfd" key="bfd_dst_ipv6">
> +          Set to an IPv6 address to set the IP address used as destination
> +          for transmitted BFD packets.
> +        </column>
> +
>          <column name="bfd" key="oam">
>            Some tunnel protocols (such as Geneve) include a bit in the header
>            to indicate that the encapsulated packet is an OAM frame. By setting
> --
> 2.25.1
>
Mike Pattrick June 9, 2023, 9:17 p.m. UTC | #2
On Tue, Aug 2, 2022 at 4:04 PM Toms Atteka <cpp.code.lv@gmail.com> wrote:
>
> Added config options for source and destination IPv6 addresses to
> BFD packets. IPv6 address presence overrides IPv4 address and
> packets are crafted as IPv6.
>
> Added IP address values to status to ease setting up IP addresses.
>
> Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
> ---
>  lib/bfd.c            | 119 ++++++++++++++++++++++++++++++++++++++-----
>  tests/bfd.at         |  30 +++++++++++
>  vswitchd/vswitch.xml |  10 ++++
>  3 files changed, 147 insertions(+), 12 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index 9698576d0..f9f359e99 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -187,6 +187,9 @@ struct bfd {
>      ovs_be32 ip_src;              /* IPv4 source address. */
>      ovs_be32 ip_dst;              /* IPv4 destination address. */
>
> +    struct in6_addr ipv6_src;              /* IPv6 source address. */
> +    struct in6_addr ipv6_dst;              /* IPv6 destination address. */

Could we just change the IP members to in6_addr and then use the
in6_addr_*_ipv4() functions? Are separate members required here?

Nit: The comments here could be aligned.

> +
>      uint16_t udp_src;             /* UDP source port. */
>
>      /* All timers in milliseconds. */
> @@ -244,10 +247,14 @@ static struct hmap *const all_bfds OVS_GUARDED_BY(mutex) = &all_bfds__;
>
>  static void bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *ip)
>      OVS_REQUIRES(mutex);
> +static void bfd_lookup_ipv6(const char *host_name, struct in6_addr *ipv6)
> +    OVS_REQUIRES(mutex);
>  static bool bfd_forwarding__(struct bfd *) OVS_REQUIRES(mutex);
>  static bool bfd_in_poll(const struct bfd *) OVS_REQUIRES(mutex);
>  static void bfd_poll(struct bfd *bfd) OVS_REQUIRES(mutex);
>  static const char *bfd_diag_str(enum diag) OVS_REQUIRES(mutex);
> +static const char *bfd_ip_str(ovs_be32 ip) OVS_REQUIRES(mutex);
> +static const char *bfd_ipv6_str(struct in6_addr ipv6) OVS_REQUIRES(mutex);
>  static const char *bfd_state_str(enum state) OVS_REQUIRES(mutex);
>  static long long int bfd_min_tx(const struct bfd *) OVS_REQUIRES(mutex);
>  static long long int bfd_tx_interval(const struct bfd *)
> @@ -332,6 +339,18 @@ bfd_get_status(const struct bfd *bfd, struct smap *smap)
>      smap_add_format(smap, "flap_count", "%"PRIu64, bfd->flap_count);
>      smap_add(smap, "remote_state", bfd_state_str(bfd->rmt_state));
>      smap_add(smap, "remote_diagnostic", bfd_diag_str(bfd->rmt_diag));
> +    if (bfd->ip_src) {
> +        smap_add(smap, "bfd_src_ip", bfd_ip_str(bfd->ip_src));
> +    }
> +    if (bfd->ip_dst) {
> +        smap_add(smap, "bfd_dst_ip", bfd_ip_str(bfd->ip_dst));
> +    }
> +    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
> +       smap_add(smap, "bfd_src_ipv6", bfd_ipv6_str(bfd->ipv6_src));
> +    }
> +    if (ipv6_addr_is_set(&bfd->ipv6_dst)) {
> +        smap_add(smap, "bfd_dst_ipv6", bfd_ipv6_str(bfd->ipv6_dst));
> +    }
>      ovs_mutex_unlock(&mutex);
>  }
>
> @@ -465,6 +484,10 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
>      bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
>                    htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst);
>
> +    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_src_ipv6", ""), &bfd->ipv6_src);
> +    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_dst_ipv6", ""), &bfd->ipv6_dst);

Other similar options in OVS use the same key for ipv4 and ipv6. For
example, the remote_ip field in netdev-vport.c. Can we do the same
thing here?

> +
> +

Nit: Extra space

>      forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
>      if (bfd->forwarding_if_rx != forwarding_if_rx) {
>          bfd->forwarding_if_rx = forwarding_if_rx;
> @@ -588,6 +611,7 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
>      struct udp_header *udp;
>      struct eth_header *eth;
>      struct ip_header *ip;
> +    struct ovs_16aligned_ip6_hdr *ip6;
>      struct msg *msg;
>
>      ovs_mutex_lock(&mutex);
> @@ -611,23 +635,40 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
>          ? eth_src : bfd->local_eth_src;
>      eth->eth_dst = eth_addr_is_zero(bfd->local_eth_dst)
>          ? eth_addr_bfd : bfd->local_eth_dst;
> -    eth->eth_type = htons(ETH_TYPE_IP);
> -
> -    ip = dp_packet_put_zeros(p, sizeof *ip);
> -    ip->ip_ihl_ver = IP_IHL_VER(5, 4);
> -    ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
> -    ip->ip_ttl = MAXTTL;
> -    ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
> -    ip->ip_proto = IPPROTO_UDP;
> -    put_16aligned_be32(&ip->ip_src, bfd->ip_src);
> -    put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
> -    /* Checksum has already been zeroed by put_zeros call. */
> -    ip->ip_csum = csum(ip, sizeof *ip);
> +
> +    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
> +        eth->eth_type = htons(ETH_TYPE_IPV6);
> +
> +        ip6 = dp_packet_put_zeros(p, sizeof *ip6);
> +
> +        ip6->ip6_vfc = 0x60;
> +        ip6->ip6_hlim = MAXTTL;
> +        ip6->ip6_nxt = IPPROTO_UDP;
> +        ip6->ip6_plen = htons(sizeof *udp + sizeof *msg);
> +
> +        memcpy(&ip6->ip6_src, &bfd->ipv6_src, sizeof(bfd->ipv6_src));
> +        memcpy(&ip6->ip6_dst, &bfd->ipv6_dst, sizeof(bfd->ipv6_dst));
> +
> +    } else {
> +        eth->eth_type = htons(ETH_TYPE_IP);
> +
> +        ip = dp_packet_put_zeros(p, sizeof *ip);
> +        ip->ip_ihl_ver = IP_IHL_VER(5, 4);
> +        ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
> +        ip->ip_ttl = MAXTTL;
> +        ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
> +        ip->ip_proto = IPPROTO_UDP;
> +        put_16aligned_be32(&ip->ip_src, bfd->ip_src);
> +        put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
> +        /* Checksum has already been zeroed by put_zeros call. */
> +        ip->ip_csum = csum(ip, sizeof *ip);
> +    }
>
>      udp = dp_packet_put_zeros(p, sizeof *udp);
>      udp->udp_src = htons(bfd->udp_src);
>      udp->udp_dst = htons(BFD_DEST_PORT);
>      udp->udp_len = htons(sizeof *udp + sizeof *msg);
> +    udp->udp_csum = htons(0xffff);

This looks odd to me. Before the checksum was set to zero indicating
no checksum, which is only valid for IPv4. But isn't an actual
checksum required for IPv6?

I actually haven't actually tested this, but it does apply cleanly.
Let me know if I'm missing something on the checksum.


Thanks,
M

>
>      msg = dp_packet_put_uninit(p, sizeof *msg);
>      msg->vers_diag = (BFD_VERSION << 5) | bfd->diag;
> @@ -674,6 +715,27 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow,
>          }
>      }
>
> +    if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> +        memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> +        if (flow->nw_proto == IPPROTO_UDP
> +            && !(flow->nw_frag & FLOW_NW_FRAG_LATER)
> +            && tp_dst_equals(flow, BFD_DEST_PORT, wc)
> +            && ipv6_addr_equals(&bfd->ipv6_src, &flow->ipv6_dst)) {
> +
> +            memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
> +
> +            bool check_tnl_key;
> +
> +            atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key);
> +            if (check_tnl_key) {
> +                memset(&wc->masks.tunnel.tun_id, 0xff,
> +                       sizeof wc->masks.tunnel.tun_id);
> +                return flow->tunnel.tun_id == htonll(0);
> +            }
> +            return true;
> +        }
> +    }
> +
>      if (flow->dl_type == htons(ETH_TYPE_IP)) {
>          memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>          if (flow->nw_proto == IPPROTO_UDP
> @@ -958,6 +1020,17 @@ bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *addr)
>      *addr = def;
>  }
>
> +static void
> +bfd_lookup_ipv6(const char *host_name, struct in6_addr *addr)
> +{
> +    if (host_name[0]) {
> +        if (ipv6_parse(host_name, addr)) {
> +            return;
> +        }
> +        VLOG_ERR_RL(&rl, "\"%s\" is not a valid IPv6 address", host_name);
> +    }
> +}
> +
>  static bool
>  bfd_in_poll(const struct bfd *bfd) OVS_REQUIRES(mutex)
>  {
> @@ -1083,6 +1156,28 @@ bfd_diag_str(enum diag diag) {
>      }
>  };
>
> +static const char *bfd_ip_str(ovs_be32 ip) {
> +    static char buf[INET_ADDRSTRLEN];
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +    ds_put_format(&ds, IP_FMT, IP_ARGS(ip));
> +
> +    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
> +    ds_destroy(&ds);
> +    return buf;
> +}
> +
> +static const char *bfd_ipv6_str(struct in6_addr ipv6) {
> +    static char buf[INET6_ADDRSTRLEN];
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +    ipv6_format_addr(&ipv6, &ds);
> +
> +    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
> +    ds_destroy(&ds);
> +    return buf;
> +}
> +
>  static void
>  log_msg(enum vlog_level level, const struct msg *p, const char *message,
>          const struct bfd *bfd) OVS_REQUIRES(mutex)
> diff --git a/tests/bfd.at b/tests/bfd.at
> index f5c6409f6..14aac0a19 100644
> --- a/tests/bfd.at
> +++ b/tests/bfd.at
> @@ -1133,3 +1133,33 @@ Datapath actions: 100
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +AT_SETUP([bfd - ipv6])
> +OVS_VSWITCHD_START(
> +  [add-br br1 -- \
> +   set bridge br1 datapath-type=dummy -- \
> +])
> +
> +AT_CHECK([ovs-vsctl add-port br0 p1 -- \
> +   set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::1 bfd:bfd_dst_ipv6=fe80::2 -- \
> +])
> +
> +AT_CHECK([ovs-vsctl add-port br1 p2 -- \
> +   set interface p2 type=dummy options:stream=unix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::2 bfd:bfd_dst_ipv6=fe80::1 -- \
> +])
> +
> +ovs-appctl time/stop
> +ovs-appctl time/warp 4100 100
> +
> +# Forwarding should be true
> +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
> +BFD_CHECK([p2], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
> +
> +# Disable bfd on p2, forwarding on p1 should go to false
> +AT_CHECK([ovs-vsctl set interface p2 bfd:enable=false])
> +
> +ovs-appctl time/warp 5000 100
> +BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index cc1dd77ec..e4a8d8d6d 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3964,6 +3964,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>            for transmitted BFD packets.  The default is <code>169.254.1.0</code>.
>          </column>
>
> +        <column name="bfd" key="bfd_src_ipv6">
> +          Set to an IPv6 address to set the IP address used as source for
> +          transmitted BFD packets.
> +        </column>
> +
> +        <column name="bfd" key="bfd_dst_ipv6">
> +          Set to an IPv6 address to set the IP address used as destination
> +          for transmitted BFD packets.
> +        </column>
> +
>          <column name="bfd" key="oam">
>            Some tunnel protocols (such as Geneve) include a bit in the header
>            to indicate that the encapsulated packet is an OAM frame. By setting
> --
> 2.25.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/lib/bfd.c b/lib/bfd.c
index 9698576d0..f9f359e99 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -187,6 +187,9 @@  struct bfd {
     ovs_be32 ip_src;              /* IPv4 source address. */
     ovs_be32 ip_dst;              /* IPv4 destination address. */
 
+    struct in6_addr ipv6_src;              /* IPv6 source address. */
+    struct in6_addr ipv6_dst;              /* IPv6 destination address. */
+
     uint16_t udp_src;             /* UDP source port. */
 
     /* All timers in milliseconds. */
@@ -244,10 +247,14 @@  static struct hmap *const all_bfds OVS_GUARDED_BY(mutex) = &all_bfds__;
 
 static void bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *ip)
     OVS_REQUIRES(mutex);
+static void bfd_lookup_ipv6(const char *host_name, struct in6_addr *ipv6)
+    OVS_REQUIRES(mutex);
 static bool bfd_forwarding__(struct bfd *) OVS_REQUIRES(mutex);
 static bool bfd_in_poll(const struct bfd *) OVS_REQUIRES(mutex);
 static void bfd_poll(struct bfd *bfd) OVS_REQUIRES(mutex);
 static const char *bfd_diag_str(enum diag) OVS_REQUIRES(mutex);
+static const char *bfd_ip_str(ovs_be32 ip) OVS_REQUIRES(mutex);
+static const char *bfd_ipv6_str(struct in6_addr ipv6) OVS_REQUIRES(mutex);
 static const char *bfd_state_str(enum state) OVS_REQUIRES(mutex);
 static long long int bfd_min_tx(const struct bfd *) OVS_REQUIRES(mutex);
 static long long int bfd_tx_interval(const struct bfd *)
@@ -332,6 +339,18 @@  bfd_get_status(const struct bfd *bfd, struct smap *smap)
     smap_add_format(smap, "flap_count", "%"PRIu64, bfd->flap_count);
     smap_add(smap, "remote_state", bfd_state_str(bfd->rmt_state));
     smap_add(smap, "remote_diagnostic", bfd_diag_str(bfd->rmt_diag));
+    if (bfd->ip_src) {
+        smap_add(smap, "bfd_src_ip", bfd_ip_str(bfd->ip_src));
+    }
+    if (bfd->ip_dst) {
+        smap_add(smap, "bfd_dst_ip", bfd_ip_str(bfd->ip_dst));
+    }
+    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
+       smap_add(smap, "bfd_src_ipv6", bfd_ipv6_str(bfd->ipv6_src));
+    }
+    if (ipv6_addr_is_set(&bfd->ipv6_dst)) {
+        smap_add(smap, "bfd_dst_ipv6", bfd_ipv6_str(bfd->ipv6_dst));
+    }
     ovs_mutex_unlock(&mutex);
 }
 
@@ -465,6 +484,10 @@  bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
     bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
                   htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst);
 
+    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_src_ipv6", ""), &bfd->ipv6_src);
+    bfd_lookup_ipv6(smap_get_def(cfg, "bfd_dst_ipv6", ""), &bfd->ipv6_dst);
+
+
     forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
     if (bfd->forwarding_if_rx != forwarding_if_rx) {
         bfd->forwarding_if_rx = forwarding_if_rx;
@@ -588,6 +611,7 @@  bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
     struct udp_header *udp;
     struct eth_header *eth;
     struct ip_header *ip;
+    struct ovs_16aligned_ip6_hdr *ip6;
     struct msg *msg;
 
     ovs_mutex_lock(&mutex);
@@ -611,23 +635,40 @@  bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
         ? eth_src : bfd->local_eth_src;
     eth->eth_dst = eth_addr_is_zero(bfd->local_eth_dst)
         ? eth_addr_bfd : bfd->local_eth_dst;
-    eth->eth_type = htons(ETH_TYPE_IP);
-
-    ip = dp_packet_put_zeros(p, sizeof *ip);
-    ip->ip_ihl_ver = IP_IHL_VER(5, 4);
-    ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
-    ip->ip_ttl = MAXTTL;
-    ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
-    ip->ip_proto = IPPROTO_UDP;
-    put_16aligned_be32(&ip->ip_src, bfd->ip_src);
-    put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
-    /* Checksum has already been zeroed by put_zeros call. */
-    ip->ip_csum = csum(ip, sizeof *ip);
+
+    if (ipv6_addr_is_set(&bfd->ipv6_src)) {
+        eth->eth_type = htons(ETH_TYPE_IPV6);
+
+        ip6 = dp_packet_put_zeros(p, sizeof *ip6);
+
+        ip6->ip6_vfc = 0x60;
+        ip6->ip6_hlim = MAXTTL;
+        ip6->ip6_nxt = IPPROTO_UDP;
+        ip6->ip6_plen = htons(sizeof *udp + sizeof *msg);
+
+        memcpy(&ip6->ip6_src, &bfd->ipv6_src, sizeof(bfd->ipv6_src));
+        memcpy(&ip6->ip6_dst, &bfd->ipv6_dst, sizeof(bfd->ipv6_dst));
+
+    } else {
+        eth->eth_type = htons(ETH_TYPE_IP);
+
+        ip = dp_packet_put_zeros(p, sizeof *ip);
+        ip->ip_ihl_ver = IP_IHL_VER(5, 4);
+        ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
+        ip->ip_ttl = MAXTTL;
+        ip->ip_tos = IPTOS_PREC_INTERNETCONTROL;
+        ip->ip_proto = IPPROTO_UDP;
+        put_16aligned_be32(&ip->ip_src, bfd->ip_src);
+        put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
+        /* Checksum has already been zeroed by put_zeros call. */
+        ip->ip_csum = csum(ip, sizeof *ip);
+    }
 
     udp = dp_packet_put_zeros(p, sizeof *udp);
     udp->udp_src = htons(bfd->udp_src);
     udp->udp_dst = htons(BFD_DEST_PORT);
     udp->udp_len = htons(sizeof *udp + sizeof *msg);
+    udp->udp_csum = htons(0xffff);
 
     msg = dp_packet_put_uninit(p, sizeof *msg);
     msg->vers_diag = (BFD_VERSION << 5) | bfd->diag;
@@ -674,6 +715,27 @@  bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow,
         }
     }
 
+    if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+        memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
+        if (flow->nw_proto == IPPROTO_UDP
+            && !(flow->nw_frag & FLOW_NW_FRAG_LATER)
+            && tp_dst_equals(flow, BFD_DEST_PORT, wc)
+            && ipv6_addr_equals(&bfd->ipv6_src, &flow->ipv6_dst)) {
+
+            memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
+
+            bool check_tnl_key;
+
+            atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key);
+            if (check_tnl_key) {
+                memset(&wc->masks.tunnel.tun_id, 0xff,
+                       sizeof wc->masks.tunnel.tun_id);
+                return flow->tunnel.tun_id == htonll(0);
+            }
+            return true;
+        }
+    }
+
     if (flow->dl_type == htons(ETH_TYPE_IP)) {
         memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
         if (flow->nw_proto == IPPROTO_UDP
@@ -958,6 +1020,17 @@  bfd_lookup_ip(const char *host_name, ovs_be32 def, ovs_be32 *addr)
     *addr = def;
 }
 
+static void
+bfd_lookup_ipv6(const char *host_name, struct in6_addr *addr)
+{
+    if (host_name[0]) {
+        if (ipv6_parse(host_name, addr)) {
+            return;
+        }
+        VLOG_ERR_RL(&rl, "\"%s\" is not a valid IPv6 address", host_name);
+    }
+}
+
 static bool
 bfd_in_poll(const struct bfd *bfd) OVS_REQUIRES(mutex)
 {
@@ -1083,6 +1156,28 @@  bfd_diag_str(enum diag diag) {
     }
 };
 
+static const char *bfd_ip_str(ovs_be32 ip) {
+    static char buf[INET_ADDRSTRLEN];
+    struct ds ds = DS_EMPTY_INITIALIZER;
+
+    ds_put_format(&ds, IP_FMT, IP_ARGS(ip));
+
+    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
+    ds_destroy(&ds);
+    return buf;
+}
+
+static const char *bfd_ipv6_str(struct in6_addr ipv6) {
+    static char buf[INET6_ADDRSTRLEN];
+    struct ds ds = DS_EMPTY_INITIALIZER;
+
+    ipv6_format_addr(&ipv6, &ds);
+
+    ovs_strlcpy(buf, ds_cstr(&ds), sizeof buf);
+    ds_destroy(&ds);
+    return buf;
+}
+
 static void
 log_msg(enum vlog_level level, const struct msg *p, const char *message,
         const struct bfd *bfd) OVS_REQUIRES(mutex)
diff --git a/tests/bfd.at b/tests/bfd.at
index f5c6409f6..14aac0a19 100644
--- a/tests/bfd.at
+++ b/tests/bfd.at
@@ -1133,3 +1133,33 @@  Datapath actions: 100
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([bfd - ipv6])
+OVS_VSWITCHD_START(
+  [add-br br1 -- \
+   set bridge br1 datapath-type=dummy -- \
+])
+
+AT_CHECK([ovs-vsctl add-port br0 p1 -- \
+   set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::1 bfd:bfd_dst_ipv6=fe80::2 -- \
+])
+
+AT_CHECK([ovs-vsctl add-port br1 p2 -- \
+   set interface p2 type=dummy options:stream=unix:$OVS_RUNDIR/p0.sock bfd:enable=true bfd:bfd_src_ipv6=fe80::2 bfd:bfd_dst_ipv6=fe80::1 -- \
+])
+
+ovs-appctl time/stop
+ovs-appctl time/warp 4100 100
+
+# Forwarding should be true
+BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
+BFD_CHECK([p2], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
+
+# Disable bfd on p2, forwarding on p1 should go to false
+AT_CHECK([ovs-vsctl set interface p2 bfd:enable=false])
+
+ovs-appctl time/warp 5000 100
+BFD_CHECK([p1], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index cc1dd77ec..e4a8d8d6d 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3964,6 +3964,16 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
           for transmitted BFD packets.  The default is <code>169.254.1.0</code>.
         </column>
 
+        <column name="bfd" key="bfd_src_ipv6">
+          Set to an IPv6 address to set the IP address used as source for
+          transmitted BFD packets.
+        </column>
+
+        <column name="bfd" key="bfd_dst_ipv6">
+          Set to an IPv6 address to set the IP address used as destination
+          for transmitted BFD packets.
+        </column>
+
         <column name="bfd" key="oam">
           Some tunnel protocols (such as Geneve) include a bit in the header
           to indicate that the encapsulated packet is an OAM frame. By setting