From 4e805597631ed0dcbdc0eecfe9d532cb75180ae7 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Tue, 16 Jul 2019 16:50:02 +0200 Subject: [PATCH] Improve multicast related defaults * use multicast only for participant discovery if using a WiFi network * default to using unicast for retransmits Signed-off-by: Erik Boasson --- src/core/ddsi/include/dds/ddsi/q_config.h | 3 +- src/core/ddsi/include/dds/ddsi/q_nwif.h | 1 + src/core/ddsi/src/q_config.c | 31 ++++-- src/core/ddsi/src/q_init.c | 17 ++++ src/core/ddsi/src/q_nwif.c | 13 +++ src/ddsrt/include/dds/ddsrt/ifaddrs.h | 7 ++ src/ddsrt/src/ifaddrs/lwip/ifaddrs.c | 1 + src/ddsrt/src/ifaddrs/posix/ifaddrs.c | 111 +++++++++++++++++++++- src/ddsrt/src/ifaddrs/windows/ifaddrs.c | 16 ++++ 9 files changed, 190 insertions(+), 10 deletions(-) diff --git a/src/core/ddsi/include/dds/ddsi/q_config.h b/src/core/ddsi/include/dds/ddsi/q_config.h index 0e3e2de..ae718e1 100644 --- a/src/core/ddsi/include/dds/ddsi/q_config.h +++ b/src/core/ddsi/include/dds/ddsi/q_config.h @@ -184,7 +184,7 @@ struct prune_deleted_ppant { int enforce_delay; }; -/* allow multicast bits: */ +/* allow multicast bits (default depends on network type): */ #define AMC_FALSE 0u #define AMC_SPDP 1u #define AMC_ASM 2u @@ -194,6 +194,7 @@ struct prune_deleted_ppant { #else #define AMC_TRUE (AMC_SPDP | AMC_ASM) #endif +#define AMC_DEFAULT 0x80000000u /* FIXME: this should be fully dynamic ... but this is easier for a quick hack */ enum transport_selector { diff --git a/src/core/ddsi/include/dds/ddsi/q_nwif.h b/src/core/ddsi/include/dds/ddsi/q_nwif.h index 5fa789b..5ca0c38 100644 --- a/src/core/ddsi/include/dds/ddsi/q_nwif.h +++ b/src/core/ddsi/include/dds/ddsi/q_nwif.h @@ -28,6 +28,7 @@ struct nn_interface { nn_locator_t netmask; uint32_t if_index; unsigned mc_capable: 1; + unsigned mc_flaky: 1; unsigned point_to_point: 1; char *name; }; diff --git a/src/core/ddsi/src/q_config.c b/src/core/ddsi/src/q_config.c index 99b697c..88dc26b 100644 --- a/src/core/ddsi/src/q_config.c +++ b/src/core/ddsi/src/q_config.c @@ -234,15 +234,16 @@ static const struct cfgelem general_cfgelems[] = { BLURB("

This element allows explicitly overruling the network address DDSI2E advertises in the discovery protocol, which by default is the address of the preferred network interface (General/NetworkInterfaceAddress), to allow DDSI2E to communicate across a Network Address Translation (NAT) device.

") }, { LEAF("ExternalNetworkMask"), 1, "0.0.0.0", ABSOFF(externalMaskString), 0, uf_string, ff_free, pf_string, BLURB("

This element specifies the network mask of the external network address. This element is relevant only when an external network address (General/ExternalNetworkAddress) is explicitly configured. In this case locators received via the discovery protocol that are within the same external subnet (as defined by this mask) will be translated to an internal address by replacing the network portion of the external address with the corresponding portion of the preferred network interface address. This option is IPv4-only.

") }, - { LEAF("AllowMulticast"), 1, "true", ABSOFF(allowMulticast), 0, uf_allow_multicast, 0, pf_allow_multicast, + { LEAF("AllowMulticast"), 1, "default", ABSOFF(allowMulticast), 0, uf_allow_multicast, 0, pf_allow_multicast, BLURB("

This element controls whether DDSI2E uses multicasts for data traffic.

\n\ -

It is a comma-separated list of some of the following keywords: \"spdp\", \"asm\", \"ssm\", or either of \"false\" or \"true\".

\n\ +

It is a comma-separated list of some of the following keywords: \"spdp\", \"asm\", \"ssm\", or either of \"false\" or \"true\", or \"default\".

\n\ \n\ -

When set to \"false\" all multicasting is disabled. The default, \"true\" enables full use of multicasts. Listening for multicasts can be controlled by General/MulticastRecvNetworkInterfaceAddresses.

") }, +

When set to \"false\" all multicasting is disabled. The default, \"true\" enables full use of multicasts. Listening for multicasts can be controlled by General/MulticastRecvNetworkInterfaceAddresses.

\n\ +

\"default\" maps on spdp if the network is a WiFi network, on true if it is a wired network

") }, { LEAF("PreferMulticast"), 1, "false", ABSOFF(prefer_multicast), 0, uf_boolean, 0, pf_boolean, BLURB("

When false (default) Cyclone DDS uses unicast for data whenever there a single unicast suffices. Setting this to true makes it prefer multicasting data, falling back to unicast only when no multicast address is available.

") }, { LEAF("MulticastTimeToLive"), 1, "32", ABSOFF(multicast_ttl), 0, uf_natint_255, 0, pf_int, @@ -532,12 +533,12 @@ static const struct cfgelem unsupp_cfgelems[] = { BLURB("

This elements configures the maximum number of DCPS domain participants this DDSI2E instance is willing to service. 0 is unlimited.

") }, { LEAF("AccelerateRexmitBlockSize"), 1, "0", ABSOFF(accelerate_rexmit_block_size), 0, uf_uint, 0, pf_uint, BLURB("

Proxy readers that are assumed to sill be retrieving historical data get this many samples retransmitted when they NACK something, even if some of these samples have sequence numbers outside the set covered by the NACK.

") }, - { LEAF("RetransmitMerging"), 1, "adaptive", ABSOFF(retransmit_merging), 0, uf_retransmit_merging, 0, pf_retransmit_merging, + { LEAF("RetransmitMerging"), 1, "never", ABSOFF(retransmit_merging), 0, uf_retransmit_merging, 0, pf_retransmit_merging, BLURB("

This elements controls the addressing and timing of retransmits. Possible values are:

\n\ \n\ -

The default is adaptive. See also Internal/RetransmitMergingPeriod.

") }, +

The default is never. See also Internal/RetransmitMergingPeriod.

") }, { LEAF("RetransmitMergingPeriod"), 1, "5 ms", ABSOFF(retransmit_merging_period), 0, uf_duration_us_1s, 0, pf_duration, BLURB("

This setting determines the size of the time window in which a NACK of some sample is ignored because a retransmit of that sample has been multicasted too recently. This setting has no effect on unicasted retransmits.

\n\

See also Internal/RetransmitMerging.

") }, @@ -1760,13 +1761,29 @@ static const uint32_t allow_multicast_codes[] = { AMC_FALSE, AMC_SPDP, AMC_ASM, static int uf_allow_multicast (struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, UNUSED_ARG(int first), const char *value) { uint32_t * const elem = cfg_address (cfgst, parent, cfgelem); - return do_uint32_bitset (cfgst, elem, allow_multicast_names, allow_multicast_codes, value); + if (ddsrt_strcasecmp (value, "default") == 0) + { + *elem = AMC_DEFAULT; + return 1; + } + else + { + *elem = 0; + return do_uint32_bitset (cfgst, elem, allow_multicast_names, allow_multicast_codes, value); + } } static void pf_allow_multicast(struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, uint32_t sources) { uint32_t *p = cfg_address (cfgst, parent, cfgelem); - do_print_uint32_bitset (cfgst, *p, sizeof (allow_multicast_codes) / sizeof (*allow_multicast_codes), allow_multicast_names, allow_multicast_codes, sources, ""); + if (*p == AMC_DEFAULT) + { + cfg_logelem (cfgst, sources, "default"); + } + else + { + do_print_uint32_bitset (cfgst, *p, sizeof (allow_multicast_codes) / sizeof (*allow_multicast_codes), allow_multicast_names, allow_multicast_codes, sources, ""); + } } static int uf_maybe_int32 (struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, UNUSED_ARG (int first), const char *value) diff --git a/src/core/ddsi/src/q_init.c b/src/core/ddsi/src/q_init.c index 9832ab2..38b6e9c 100644 --- a/src/core/ddsi/src/q_init.c +++ b/src/core/ddsi/src/q_init.c @@ -936,7 +936,24 @@ int rtps_init (void) config.participantIndex = PARTICIPANT_INDEX_AUTO; mc_available = false; } + else if (config.allowMulticast & AMC_DEFAULT) + { + /* default is dependent on network interface type: if multicast is believed to be flaky, + use multicast only for SPDP packets */ + assert ((config.allowMulticast & ~AMC_DEFAULT) == 0); + if (gv.interfaces[gv.selected_interface].mc_flaky) + { + config.allowMulticast = AMC_SPDP; + DDS_LOG(DDS_LC_CONFIG, "presumed flaky multicast, use for SPDP only\n"); + } + else + { + DDS_LOG(DDS_LC_CONFIG, "presumed robust multicast support, use for everything\n"); + config.allowMulticast = AMC_TRUE; + } + } } + assert ((config.allowMulticast & AMC_DEFAULT) == 0); if (set_recvips () < 0) goto err_set_recvips; if (set_spdp_address () < 0) diff --git a/src/core/ddsi/src/q_nwif.c b/src/core/ddsi/src/q_nwif.c index 87b2acd..a6a758e 100644 --- a/src/core/ddsi/src/q_nwif.c +++ b/src/core/ddsi/src/q_nwif.c @@ -485,6 +485,18 @@ int find_own_ip (const char *requested_address) continue; } + switch (ifa->type) + { + case DDSRT_IFTYPE_WIFI: + DDS_LOG(DDS_LC_CONFIG, " wireless"); + break; + case DDSRT_IFTYPE_WIRED: + DDS_LOG(DDS_LC_CONFIG, " wired"); + break; + case DDSRT_IFTYPE_UNKNOWN: + break; + } + #if defined(__linux) && !LWIP_SOCKET if (ifa->addr->sa_family == AF_PACKET) { @@ -571,6 +583,7 @@ int find_own_ip (const char *requested_address) memset(&gv.interfaces[gv.n_interfaces].netmask.address, 0, sizeof(gv.interfaces[gv.n_interfaces].netmask.address)); } gv.interfaces[gv.n_interfaces].mc_capable = ((ifa->flags & IFF_MULTICAST) != 0); + gv.interfaces[gv.n_interfaces].mc_flaky = ((ifa->type == DDSRT_IFTYPE_WIFI) != 0); gv.interfaces[gv.n_interfaces].point_to_point = ((ifa->flags & IFF_POINTOPOINT) != 0); gv.interfaces[gv.n_interfaces].if_index = ifa->index; gv.interfaces[gv.n_interfaces].name = ddsrt_strdup (if_name); diff --git a/src/ddsrt/include/dds/ddsrt/ifaddrs.h b/src/ddsrt/include/dds/ddsrt/ifaddrs.h index 389356e..3eea634 100644 --- a/src/ddsrt/include/dds/ddsrt/ifaddrs.h +++ b/src/ddsrt/include/dds/ddsrt/ifaddrs.h @@ -18,11 +18,18 @@ extern "C" { #endif +enum ddsrt_iftype { + DDSRT_IFTYPE_UNKNOWN, + DDSRT_IFTYPE_WIRED, + DDSRT_IFTYPE_WIFI +}; + struct ddsrt_ifaddrs { struct ddsrt_ifaddrs *next; char *name; uint32_t index; uint32_t flags; + enum ddsrt_iftype type; struct sockaddr *addr; struct sockaddr *netmask; struct sockaddr *broadaddr; diff --git a/src/ddsrt/src/ifaddrs/lwip/ifaddrs.c b/src/ddsrt/src/ifaddrs/lwip/ifaddrs.c index 65985f5..68b4401 100644 --- a/src/ddsrt/src/ifaddrs/lwip/ifaddrs.c +++ b/src/ddsrt/src/ifaddrs/lwip/ifaddrs.c @@ -95,6 +95,7 @@ copyaddr( } else { ifa->flags = getflags(netif, addr); ifa->index = netif->num; + ifa->type = DDSRT_IFTYPE_UNKNOWN; if (IP_IS_V4(addr)) { static const size_t sz = sizeof(struct sockaddr_in); diff --git a/src/ddsrt/src/ifaddrs/posix/ifaddrs.c b/src/ddsrt/src/ifaddrs/posix/ifaddrs.c index 208c4e0..8bf8f52 100644 --- a/src/ddsrt/src/ifaddrs/posix/ifaddrs.c +++ b/src/ddsrt/src/ifaddrs/posix/ifaddrs.c @@ -21,8 +21,113 @@ extern const int *const os_supp_afs; +#if defined __linux +#include + +static enum ddsrt_iftype guess_iftype (const struct ifaddrs *sys_ifa) +{ + FILE *fp; + char ifnam[IFNAMSIZ + 1]; /* not sure whether IFNAMSIZ includes a terminating 0, can't be bothered */ + int c; + size_t np; + enum ddsrt_iftype type = DDSRT_IFTYPE_UNKNOWN; + if ((fp = fopen ("/proc/net/wireless", "r")) == NULL) + return type; + /* expected format: + :Inter-| sta-| Quality | Discarded packets | Missed | WE + : face | tus | link level noise | nwid crypt frag retry misc | beacon | 22 + : wlan0: 0000 67. -43. -256 0 0 0 0 0 0 + (where : denotes the start of the line) + + SKIP_HEADER_1 skips up to and including the first newline; then SKIP_TO_EOL skips + up to and including the second newline, so the first line that gets interpreted is + the third. + */ + enum { SKIP_HEADER_1, SKIP_WHITE, READ_NAME, SKIP_TO_EOL } state = SKIP_HEADER_1; + np = 0; + while (type != DDSRT_IFTYPE_WIFI && (c = fgetc (fp)) != EOF) { + switch (state) { + case SKIP_HEADER_1: + if (c == '\n') { + state = SKIP_TO_EOL; + } + break; + case SKIP_WHITE: + if (c != ' ' && c != '\t') { + ifnam[np++] = (char) c; + state = READ_NAME; + } + break; + case READ_NAME: + if (c == ':') { + ifnam[np] = 0; + if (strcmp (ifnam, sys_ifa->ifa_name) == 0) + type = DDSRT_IFTYPE_WIFI; + state = SKIP_TO_EOL; + np = 0; + } else if (np < sizeof (ifnam) - 1) { + ifnam[np++] = (char) c; + } + break; + case SKIP_TO_EOL: + if (c == '\n') { + state = SKIP_WHITE; + } + break; + } + } + fclose (fp); + return type; +} +#elif defined __APPLE__ /* probably works for all BSDs */ +#include +#include +#include +#include + +static enum ddsrt_iftype guess_iftype (const struct ifaddrs *sys_ifa) +{ + int sock; + if ((sock = socket (sys_ifa->ifa_addr->sa_family, SOCK_DGRAM, 0)) == -1) + return DDSRT_IFTYPE_UNKNOWN; + + struct ifmediareq ifmr; + enum ddsrt_iftype type; + memset (&ifmr, 0, sizeof (ifmr)); + ddsrt_strlcpy (ifmr.ifm_name, sys_ifa->ifa_name, sizeof (ifmr.ifm_name)); + if (ioctl (sock, SIOCGIFMEDIA, (caddr_t) &ifmr) < 0) + { + type = DDSRT_IFTYPE_UNKNOWN; + } + else + { + switch (IFM_TYPE (ifmr.ifm_active)) + { + case IFM_ETHER: + case IFM_TOKEN: + case IFM_FDDI: + type = DDSRT_IFTYPE_WIRED; + break; + case IFM_IEEE80211: + type = DDSRT_IFTYPE_WIFI; + break; + default: + type = DDSRT_IFTYPE_UNKNOWN; + break; + } + } + close (sock); + return type; +} +#else +static enum ddsrt_iftype guess_iftype (const struct ifaddrs *sys_ifa) +{ + return DDSRT_IFTYPE_UNKNOWN; +} +#endif + static dds_return_t -copyaddr(ddsrt_ifaddrs_t **ifap, const struct ifaddrs *sys_ifa) +copyaddr(ddsrt_ifaddrs_t **ifap, const struct ifaddrs *sys_ifa, enum ddsrt_iftype type) { dds_return_t err = DDS_RETCODE_OK; ddsrt_ifaddrs_t *ifa; @@ -37,6 +142,7 @@ copyaddr(ddsrt_ifaddrs_t **ifap, const struct ifaddrs *sys_ifa) err = DDS_RETCODE_OUT_OF_RESOURCES; } else { ifa->index = if_nametoindex(sys_ifa->ifa_name); + ifa->type = type; ifa->flags = sys_ifa->ifa_flags; if ((ifa->name = ddsrt_strdup(sys_ifa->ifa_name)) == NULL || (ifa->addr = ddsrt_memdup(sys_ifa->ifa_addr, sz)) == NULL || @@ -109,7 +215,8 @@ ddsrt_getifaddrs( } if (use) { - err = copyaddr(&ifa_next, sys_ifa); + enum ddsrt_iftype type = guess_iftype (sys_ifa); + err = copyaddr(&ifa_next, sys_ifa, type); if (err == DDS_RETCODE_OK) { if (ifa == NULL) { ifa = ifa_root = ifa_next; diff --git a/src/ddsrt/src/ifaddrs/windows/ifaddrs.c b/src/ddsrt/src/ifaddrs/windows/ifaddrs.c index 7dcb8aa..7c5045c 100644 --- a/src/ddsrt/src/ifaddrs/windows/ifaddrs.c +++ b/src/ddsrt/src/ifaddrs/windows/ifaddrs.c @@ -152,6 +152,21 @@ getflags(const PIP_ADAPTER_ADDRESSES iface) return flags; } +static enum ddsrt_iftype +guess_iftype (const PIP_ADAPTER_ADDRESSES iface) +{ + switch (iface->IfType) { + case IF_TYPE_IEEE80211: + return DDSRT_IFTYPE_WIFI; + case IF_TYPE_ETHERNET_CSMACD: + case IF_TYPE_IEEE1394: + case IF_TYPE_ISO88025_TOKENRING: + return DDSRT_IFTYPE_WIRED; + default: + return DDSRT_IFTYPE_UNKNOWN; + } +} + static int copyaddr( ddsrt_ifaddrs_t **ifap, @@ -175,6 +190,7 @@ copyaddr( err = DDS_RETCODE_OUT_OF_RESOURCES; } else { ifa->flags = getflags(iface); + ifa->type = guess_iftype(iface); ifa->addr = ddsrt_memdup(sa, sz); (void)ddsrt_asprintf(&ifa->name, "%wS", iface->FriendlyName); if (ifa->addr == NULL || ifa->name == NULL) {