pick_address: Warn and continue when you find at least 1 IPv4 or IPv6 address

Currently if specify a single public or cluster network, yet have both
`ms bind ipv4` and `ms bind ipv6` set daemons crash when they can't find
both IPs from the same network:

    unable to find any IPv4 address in networks '2001:db8:11d::/120' interfaces ''

And rightly so, of course it can't find an IPv4 network in an IPv6
network.
This patch, adds a new helper method, networks_address_family_coverage,
that takes the list of networks and returns a bitmap of address families
supported.
We then check to see if we have enough networks defined and if you don't
it'll warn and then continue.

Also update the network-config-ref to mention having to define both
address family addresses for cluster and or public networks.

As well as a warning about `ms bind ipv4` being enabled by default which
is easy to miss, there by enabling dual stack when you may only be
expect single stack IPv6.

Thee is also a drive by to fix a `note` that wan't being displayed due
to missing RST syntax.

Signed-off-by: Matthew Oliver <moliver@suse.com>
Fixes: https://tracker.ceph.com/issues/46845
Fixes: https://tracker.ceph.com/issues/39711
(cherry picked from commit 9f75dfbf364f5140b3f291e0a2c6769bc3d8cbac)

diff --git a/doc/rados/configuration/msgr2.rst b/doc/rados/configuration/msgr2.rst
index cea412e73c6bb9e1ef9a586ad6a784be1c0c3f77..6470abe68ccf4cdfb463b64b8fd96e66b8237cb9 100644 (file)
--- a/doc/rados/configuration/msgr2.rst
+++ b/doc/rados/configuration/msgr2.rst
@@ -88,7 +88,7 @@ Similarly, two options control whether IPv4 and IPv6 addresses are used:
   * ``ms_bind_ipv6`` [default: false] controls whether a daemon binds
     to an IPv6 address
 
-.. note: The ability to bind to multiple ports has paved the way for
+.. note:: The ability to bind to multiple ports has paved the way for
    dual-stack IPv4 and IPv6 support.  That said, dual-stack support is
    not yet tested as of Nautilus v14.2.0 and likely needs some
    additional code changes to work correctly.

diff --git a/doc/rados/configuration/network-config-ref.rst b/doc/rados/configuration/network-config-ref.rst
index 5a719e3cc495d27d0ad31cbe2cf543c17d3c2b4f..c027ad40ddba1232832d39cd11690cb7adbe6d4a 100644 (file)
--- a/doc/rados/configuration/network-config-ref.rst
+++ b/doc/rados/configuration/network-config-ref.rst
@@ -201,6 +201,27 @@ following option to the ``[global]`` section of your Ceph configuration file.
 We prefer that the cluster network is **NOT** reachable from the public network
 or the Internet for added security.
 
+IPv4/IPv6 Dual Stack Mode
+-------------------------
+
+If you want to run in an IPv4/IPv6 dual stack mode and want to define your public and/or
+cluster networks, then you need to specify both your IPv4 and IPv6 networks for each:
+
+.. code-block:: ini
+
+       [global]
+               # ... elided configuration
+               public network = {IPv4 public-network/netmask}, {IPv6 public-network/netmask}
+
+This is so ceph can find a valid IP address for both address families.
+
+If you want just an IPv4 or an IPv6 stack environment, then make sure you set the `ms bind`
+options correctly.
+
+.. note::
+   Binding to IPv4 is enabled by default, so if you just add the option to bind to IPv6
+   you'll actually put yourself into dual stack mode. If you want just IPv6, then disable IPv4 and
+   enable IPv6. See `Bind`_ below.
 
 Ceph Daemons
 ============
@@ -336,11 +357,16 @@ addresses.
 :Default: ``7300``
 :Required: No. 
 
+``ms bind ipv4``
+
+:Description: Enables Ceph daemons to bind to IPv4 addresses.
+:Type: Boolean
+:Default: ``true``
+:Required: No
 
 ``ms bind ipv6``
 
-:Description: Enables Ceph daemons to bind to IPv6 addresses. Currently the
-              messenger *either* uses IPv4 or IPv6, but it cannot do both.
+:Description: Enables Ceph daemons to bind to IPv6 addresses.
 :Type: Boolean
 :Default: ``false``
 :Required: No

diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc
index e6da4248b5017c2c629b9eb14184026097ada5f2..79513bb652d5b5c5e70958c7095c738b0948869a 100644 (file)
--- a/src/common/pick_address.cc
+++ b/src/common/pick_address.cc
@@ -279,6 +279,32 @@ static int fill_in_one_address(
   return 0;
 }
 
+unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks) {
+  std::list<string> nets;
+  get_str_list(networks, nets);
+  unsigned found_ipv = 0;
+
+  for (auto& s : nets) {
+    struct sockaddr_storage net;
+    unsigned prefix_len;
+    if (!parse_network(s.c_str(), &net, &prefix_len)) {
+      lderr(cct) << "unable to parse network: " << s << dendl;
+      exit(1);
+    }
+
+    switch (net.ss_family) {
+    case AF_INET:
+      found_ipv |= CEPH_PICK_ADDRESS_IPV4;
+      break;
+    case AF_INET6:
+      found_ipv |= CEPH_PICK_ADDRESS_IPV6;
+      break;
+    }
+  }
+
+  return found_ipv;
+}
+
 int pick_addresses(
   CephContext *cct,
   unsigned flags,
@@ -352,6 +378,7 @@ int pick_addresses(
       !networks.empty()) {
     int ipv4_r = !(ipv & CEPH_PICK_ADDRESS_IPV4) ? 0 : -1;
     int ipv6_r = !(ipv & CEPH_PICK_ADDRESS_IPV6) ? 0 : -1;
+    unsigned found_ipv = networks_address_family_coverage(cct, networks);
     // first try on preferred numa node (if >= 0), then anywhere.
     while (true) {
       // note: pass in ipv to filter the matching addresses
@@ -372,6 +399,11 @@ int pick_addresses(
                                      networks, interfaces, addrs,
                                      preferred_numa_node);
       }
+      if (found_ipv != 0 && (found_ipv & ipv != ipv)) {
+        lderr(cct) << "An IP address was found, but not enough networks to cover both address families. "
+                   << "An IPv4 and IPv6 network is required for dual stack. Continuing with one stack" << dendl;
+        break;
+      }
       if (ipv4_r >= 0 && ipv6_r >= 0) {
        break;
       }

diff --git a/src/common/pick_address.h b/src/common/pick_address.h
index ba9473a9649a3751d86459f3d9d07c5ec69a5870..80e9d102c028a8bc2615a5c6ad8258c7ae540ea3 100644 (file)
--- a/src/common/pick_address.h
+++ b/src/common/pick_address.h
@@ -85,4 +85,11 @@ int get_iface_numa_node(
   const std::string& iface,
   int *node);
 
+/**
+ * Return a bitmap of address families that are covered by networks
+ *
+ * @param cct context (used for logging)
+ * @param string of networks
+ */
+unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks);
 #endif

diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc
index d904a6819fffde92ada756d041959c2d376f2284..6f6c39fcb2b2589a914b0c9929de2361dbf70465 100644 (file)
--- a/src/test/test_ipaddr.cc
+++ b/src/test/test_ipaddr.cc
@@ -931,20 +931,26 @@ TEST(pick_address, filtering)
 
 TEST(pick_address, ipv4_ipv6_enabled)
 {
-  struct ifaddrs one;
+  struct ifaddrs one, two;
   struct sockaddr_in a_one;
+  struct sockaddr_in6 a_two;
 
-  one.ifa_next = NULL;
+  one.ifa_next = &two;
   one.ifa_addr = (struct sockaddr*)&a_one;
   one.ifa_name = eth0;
 
+  two.ifa_next = NULL;
+  two.ifa_addr = (struct sockaddr*)&a_two;
+  two.ifa_name = eth0;
+
   ipv4(&a_one, "10.1.1.2");
+  ipv6(&a_two, "2001:1234:5678:90ab::cdef");
 
   CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
   cct->_conf._clear_safe_to_start_threads();  // so we can set configs
 
   cct->_conf.set_val("public_addr", "");
-  cct->_conf.set_val("public_network", "10.1.1.0/24");
+  cct->_conf.set_val("public_network", "10.1.1.0/24, 2001::/16");
   cct->_conf.set_val("public_network_interface", "");
   cct->_conf.set_val("cluster_addr", "");
   cct->_conf.set_val("cluster_network", "");
@@ -958,11 +964,15 @@ TEST(pick_address, ipv4_ipv6_enabled)
                           CEPH_PICK_ADDRESS_MSGR1,
                           &one, &av);
     cout << av << std::endl;
-    ASSERT_EQ(-1, r);
+    ASSERT_EQ(0, r);
+    // Got 2 address
+    ASSERT_EQ(2u, av.v.size());
+    ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0]));
+    ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[1]));
   }
 }
 
-TEST(pick_address, ipv4_ipv6_enabled2)
+TEST(pick_address, only_ipv6_enabled)
 {
   struct ifaddrs one;
   struct sockaddr_in6 a_one;
@@ -983,6 +993,76 @@ TEST(pick_address, ipv4_ipv6_enabled2)
   cct->_conf.set_val("cluster_network", "");
   cct->_conf.set_val("cluster_network_interface", "");
   cct->_conf.set_val("ms_bind_ipv6", "true");
+  cct->_conf.set_val("ms_bind_ipv4", "false");
+
+  entity_addrvec_t av;
+  {
+    int r = pick_addresses(cct,
+                          CEPH_PICK_ADDRESS_PUBLIC |
+                          CEPH_PICK_ADDRESS_MSGR1,
+                          &one, &av);
+    cout << av << std::endl;
+    ASSERT_EQ(0, r);
+    ASSERT_EQ(1u, av.v.size());
+    ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0]));
+  }
+}
+
+TEST(pick_address, only_ipv4_enabled)
+{
+  struct ifaddrs one;
+  struct sockaddr_in a_one;
+
+  one.ifa_next = NULL;
+  one.ifa_addr = (struct sockaddr*)&a_one;
+  one.ifa_name = eth0;
+
+  ipv4(&a_one, "10.1.1.2");
+
+  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
+  cct->_conf._clear_safe_to_start_threads();  // so we can set configs
+
+  cct->_conf.set_val("public_addr", "");
+  cct->_conf.set_val("public_network", "10.1.1.0/24");
+  cct->_conf.set_val("public_network_interface", "");
+  cct->_conf.set_val("cluster_addr", "");
+  cct->_conf.set_val("cluster_network", "");
+  cct->_conf.set_val("cluster_network_interface", "");
+  
+  entity_addrvec_t av;
+  {
+    int r = pick_addresses(cct,
+                          CEPH_PICK_ADDRESS_PUBLIC |
+                          CEPH_PICK_ADDRESS_MSGR1,
+                          &one, &av);
+    cout << av << std::endl;
+    ASSERT_EQ(0, r);
+    ASSERT_EQ(1u, av.v.size());
+    ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[0]));
+  }
+}
+
+TEST(pick_address, ipv4_ipv6_enabled_not_enough_networks)
+{
+  struct ifaddrs one;
+  struct sockaddr_in a_one;
+
+  one.ifa_next = NULL;
+  one.ifa_addr = (struct sockaddr*)&a_one;
+  one.ifa_name = eth0;
+
+  ipv4(&a_one, "10.1.1.2");
+
+  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
+  cct->_conf._clear_safe_to_start_threads();  // so we can set configs
+
+  cct->_conf.set_val("public_addr", "");
+  cct->_conf.set_val("public_network", "10.1.1.0/24");
+  cct->_conf.set_val("public_network_interface", "");
+  cct->_conf.set_val("cluster_addr", "");
+  cct->_conf.set_val("cluster_network", "");
+  cct->_conf.set_val("cluster_network_interface", "");
+  cct->_conf.set_val("ms_bind_ipv6", "true");
 
   entity_addrvec_t av;
   {
@@ -992,6 +1072,31 @@ TEST(pick_address, ipv4_ipv6_enabled2)
                           &one, &av);
     cout << av << std::endl;
     ASSERT_EQ(-1, r);
+    ASSERT_EQ(1u, av.v.size());
+    ASSERT_EQ(string("v2:10.1.1.2:0/0"), stringify(av.v[0]));
   }
 }
 
+TEST(networks_address_family_coverage, just_ipv4)
+{
+  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
+  std::string networks = "10.0.0.0/24";
+  unsigned r = networks_address_family_coverage(cct, networks);
+  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4, r);
+}
+
+TEST(networks_address_family_coverage, just_ipv6)
+{
+  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
+  std::string networks = "2001::/16";
+  unsigned r = networks_address_family_coverage(cct, networks);
+  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV6, r);
+}
+
+TEST(networks_address_family_coverage, ipv6_and_ipv4)
+{
+  CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD);
+  std::string networks = "2001::/16, 10.0.0.0/16";
+  unsigned r = networks_address_family_coverage(cct, networks);
+  ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, r);
+}