On Tue, Aug 2, 2022 at 4:04 PM Toms Atteka <[email protected]> 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 <[email protected]>
> ---
> 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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
