Hello,

Nobody has done it yet, so I've tried to implement it. The patch is
attached. Of course feel free to alter naming, wording, add credits
for the reported, etc. as you wish.

Ville, could you check that it works for you?

PS.
1) I also noticed there is "strict bind" example is missing in BFD
template config in the documentation.
2) What do you think about changing bfd flags and/or socket flags to
bit fields too, as it was done with other flags recently?

Regards,
Alexander


On Wed, Jun 12, 2024 at 3:14 PM Maria Matejka via Bird-users
<bird-users@network.cz> wrote:
>
> Hello!
>
> On Wed, Jun 12, 2024 at 06:32:10PM +0700, Ville O wrote:
>
> At least some Huawei devices use a checksum of 0 for all IPv6 BFD UDP packets 
> after finishing Poll/Final. All packets sent with states other than Up or 
> with flags other than C have the correct checksums.
>
> […]
>
> This issue could be worked around on the BIRD side at least on the Linux 
> platform. RFC6936 allows [3] for hosts to enable accepting IPv6 UDP with a 
> checksum of 0 and this is implemented in Linux kernels from 3.16 with sockopt 
> “UDP_NO_CHECK6_RX”. I have tested that this indeed works: checksum 0 packets 
> are received to AF_INET6, SOCK_DGRAM sockets when it is enabled.
>
> I wonder if it would be acceptable to enable this option on the IPv6 
> socket(s) used for BFD in BIRD, if supported by the platform?
>
> Patches welcome. It should be configurable and off by default. For more 
> information on how to contribute, see the contributing guidelines:
>
> https://gitlab.nic.cz/labs/bird/-/blob/master/CONTRIBUTING.md
>
> Thank you for raising awareness about this issue.
>
> Maria
>
> – Maria Matejka (she/her) | BIRD Team Leader | CZ.NIC, z.s.p.o.
commit 2fcc6b08a96e45d49bb2ba901e7d1873a6564312
Author: Alexander Zubkov <green@qrator.net>
Date:   Sat Jun 22 19:32:48 2024 +0200

    BFD: add option to ignore checksum for rx IPv6 UDP packets
    
    Some vendors do not set the checksum correctly for IPv6 UDP packets.
    For interopertability with such implementations one can set UDP_NO_CHECK6_RX
    socket option on Linux, in order not to verify the checksum.

diff --git a/doc/bird.sgml b/doc/bird.sgml
index 8035ec18..ac5dd8fc 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -2499,6 +2499,7 @@ milliseconds.
 <code>
 protocol bfd [&lt;name&gt;] {
 	accept [ipv4|ipv6] [direct|multihop];
+	check udp6 checksum rx &lt;switch&gt;;
 	interface &lt;interface pattern&gt; {
 		interval &lt;time&gt;;
 		min rx interval &lt;time&gt;;
@@ -2541,6 +2542,12 @@ protocol bfd [&lt;name&gt;] {
 	to configure separate BFD protocol instances for IPv4 and for IPv6
 	sessions.
 
+	<tag><label id="bfd-check-udp6-checksum-rx">check udp6 checksum rx <m/switch/</tag>
+	This options when disabled configures the rx socket not to verify
+	the IPv6 UDP checksum on the systems, which support such socket option
+	(ex. <cf>UDP_NO_CHECK6_RX</cf>). It is needed for interoperability with some
+	vendors that do not set the checksum correctly. Default: enabled.
+
 	<tag><label id="bfd-strict-bind">strict bind <m/switch/</tag>
 	Specify whether each BFD interface should use a separate listening
 	socket bound to its local address, or just use a shared listening socket
diff --git a/lib/socket.h b/lib/socket.h
index 231c10d8..4f50fb3e 100644
--- a/lib/socket.h
+++ b/lib/socket.h
@@ -131,6 +131,8 @@ extern int sk_priority_control;		/* Suggested priority for control traffic, shou
 #define SKF_HDRINCL	0x400	/* Used internally */
 #define SKF_PKTINFO	0x800	/* Used internally */
 
+#define SKF_UDP6_NO_RX_CSUM	0x1000	/* Do not verify checksum for incoming UDP6 */
+
 /*
  *	Socket types		     SA SP DA DP IF  TTL SendTo	(?=may, -=must not, *=must)
  */
diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c
index 4f8499ba..4ce4bf5a 100644
--- a/proto/bfd/bfd.c
+++ b/proto/bfd/bfd.c
@@ -1149,7 +1149,8 @@ bfd_reconfigure(struct proto *P, struct proto_config *c)
       (new->accept_ipv6 != old->accept_ipv6) ||
       (new->accept_direct != old->accept_direct) ||
       (new->accept_multihop != old->accept_multihop) ||
-      (new->strict_bind != old->strict_bind))
+      (new->strict_bind != old->strict_bind) ||
+      (new->udp6_no_rx_checksum != old->udp6_no_rx_checksum))
     return 0;
 
   birdloop_mask_wakeups(p->loop);
diff --git a/proto/bfd/bfd.h b/proto/bfd/bfd.h
index 847c6b14..dfc7d696 100644
--- a/proto/bfd/bfd.h
+++ b/proto/bfd/bfd.h
@@ -48,6 +48,7 @@ struct bfd_config
   u8 accept_direct;
   u8 accept_multihop;
   u8 strict_bind;
+  u8 udp6_no_rx_checksum;
 };
 
 struct bfd_iface_config
diff --git a/proto/bfd/config.Y b/proto/bfd/config.Y
index 39e7577f..112576e5 100644
--- a/proto/bfd/config.Y
+++ b/proto/bfd/config.Y
@@ -24,7 +24,7 @@ CF_DECLS
 CF_KEYWORDS(BFD, MIN, IDLE, RX, TX, INTERVAL, MULTIPLIER, PASSIVE,
 	INTERFACE, MULTIHOP, NEIGHBOR, DEV, LOCAL, AUTHENTICATION,
 	NONE, SIMPLE, METICULOUS, KEYED, MD5, SHA1, IPV4, IPV6, DIRECT,
-	STRICT, BIND)
+	STRICT, BIND, CHECK, UDP6, CHECKSUM)
 
 %type <iface> bfd_neigh_iface
 %type <a> bfd_neigh_local
@@ -51,6 +51,7 @@ bfd_proto_item:
  | MULTIHOP bfd_multihop
  | NEIGHBOR bfd_neighbor
  | STRICT BIND bool { BFD_CFG->strict_bind = $3; }
+ | CHECK UDP6 CHECKSUM RX bool { BFD_CFG->udp6_no_rx_checksum = ! $5; }
  ;
 
 bfd_proto_opts:
diff --git a/proto/bfd/packets.c b/proto/bfd/packets.c
index aec91ca6..f56e0b6b 100644
--- a/proto/bfd/packets.c
+++ b/proto/bfd/packets.c
@@ -416,6 +416,8 @@ bfd_err_hook(sock *sk, int err)
 sock *
 bfd_open_rx_sk(struct bfd_proto *p, int multihop, int af)
 {
+  struct bfd_config *cf = (struct bfd_config *) (p->p.cf);
+
   sock *sk = sk_new(p->tpool);
   sk->type = SK_UDP;
   sk->subtype = af;
@@ -431,6 +433,8 @@ bfd_open_rx_sk(struct bfd_proto *p, int multihop, int af)
   sk->tos = IP_PREC_INTERNET_CONTROL;
   sk->priority = sk_priority_control;
   sk->flags = SKF_THREAD | SKF_LADDR_RX | (!multihop ? SKF_TTL_RX : 0);
+  if (cf->udp6_no_rx_checksum)
+    sk->flags |= SKF_UDP6_NO_RX_CSUM;
 
   if (sk_open(sk) < 0)
     goto err;
@@ -447,6 +451,8 @@ err:
 sock *
 bfd_open_rx_sk_bound(struct bfd_proto *p, ip_addr local, struct iface *ifa)
 {
+  struct bfd_config *cf = (struct bfd_config *) (p->p.cf);
+
   sock *sk = sk_new(p->tpool);
   sk->type = SK_UDP;
   sk->saddr = local;
@@ -463,6 +469,8 @@ bfd_open_rx_sk_bound(struct bfd_proto *p, ip_addr local, struct iface *ifa)
   sk->tos = IP_PREC_INTERNET_CONTROL;
   sk->priority = sk_priority_control;
   sk->flags = SKF_THREAD | SKF_BIND | (ifa ? SKF_TTL_RX : 0);
+  if (cf->udp6_no_rx_checksum)
+    sk->flags |= SKF_UDP6_NO_RX_CSUM;
 
   if (sk_open(sk) < 0)
     goto err;
diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c
index 9b499020..8c7bf1d6 100644
--- a/sysdep/unix/io.c
+++ b/sysdep/unix/io.c
@@ -517,6 +517,21 @@ sk_set_high_port(sock *s UNUSED)
   return 0;
 }
 
+static inline int
+sk_set_udp6_no_rx_csum(sock *s)
+{
+#ifdef UDP_NO_CHECK6_RX
+  int y = 1;
+
+  if (setsockopt(s->fd, SOL_UDP, UDP_NO_CHECK6_RX, &y, sizeof(y)) < 0)
+    ERR("UDP_NO_CHECK6_RX");
+
+  return 0;
+#else
+  ERR("UDP_NO_CHECK6_RX");
+#endif
+}
+
 static inline byte *
 sk_skip_ip_header(byte *pkt, int *len)
 {
@@ -1475,6 +1490,10 @@ sk_open(sock *s)
       if (sk_set_freebind(s) < 0)
         log(L_WARN "Socket error: %s%#m", s->err);
 
+    if (s->flags & SKF_UDP6_NO_RX_CSUM && s->af == AF_INET6 && s->type == SK_UDP)
+      if (sk_set_udp6_no_rx_csum(s) < 0)
+        log(L_WARN "Socket error: %s%#m", s->err);
+
     sockaddr_fill(&sa, s->af, bind_addr, s->iface, bind_port);
     if (bind(fd, &sa.sa, SA_LEN(sa)) < 0)
       ERR2("bind");

Reply via email to