From 443bc7b797b2cebee24c881be40af36f07391b92 Mon Sep 17 00:00:00 2001 From: Alexander Pevzner Date: Fri, 23 Feb 2024 14:43:52 +0300 Subject: [PATCH] WSDD: management of extended discovery time revisited (see also #151e440) See #151e440 for problem description and workaround method being used The criterion of applying extended time was simplified. Instead of looking for unpaired WSDD/MDNS device we simply compare count of WSDD/MDNS devices, discovered so far, with problematic model names This change not only makes things conceptually simpler, but also allows to take in account MDNS-devices early, with not yet completed discovery/resolving process --- airscan-mdns.c | 68 +++++++++++++++++++++++++++++++++++++++++ airscan-wsdd.c | 52 ++++++++++++++++++++++++++++++-- airscan-zeroconf.c | 75 ++++++++++++++-------------------------------- airscan.h | 33 ++++++++++++-------- 4 files changed, 160 insertions(+), 68 deletions(-) diff --git a/airscan-mdns.c b/airscan-mdns.c index 3d97b94..5bec260 100644 --- a/airscan-mdns.c +++ b/airscan-mdns.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -70,6 +71,7 @@ typedef struct { */ static log_ctx *mdns_log; static ll_head mdns_finding_list; +static size_t mdns_finding_list_count; static const AvahiPoll *mdns_avahi_poll; static AvahiTimeout *mdns_avahi_restart_timer; static AvahiClient *mdns_avahi_client; @@ -368,6 +370,7 @@ mdns_finding_get (ZEROCONF_METHOD method, int ifindex, const char *name, mdns = mdns_finding_new(method, ifindex, name, initscan); ll_push_end(&mdns_finding_list, &mdns->node_list); + mdns_finding_list_count ++; return mdns; } @@ -401,7 +404,10 @@ mdns_finding_del (mdns_finding *mdns) if (mdns->is_published) { zeroconf_finding_withdraw(&mdns->finding); } + ll_del(&mdns->node_list); + mdns_finding_list_count --; + mdns_finding_kill_resolvers(mdns); mdns_finding_free(mdns); } @@ -1281,6 +1287,68 @@ mdns_query_get_ptr (const mdns_query *query) return query->ptr; } +/***** Miscellaneous *****/ + +/* mdns_device_count_by_model returns count of distinct devices + * with model names matching the specified parent. + * + * Several instances of the same device (i.e. printer vs scanner) are + * counted only once per network interface. + * + * WSDD uses this function to decide when to use extended discovery + * time (some devices are known to be hard for WD-Discovery) + * + * Pattern is the glob-style expression, applied to the model name + * of discovered devices. + */ +unsigned int +mdns_device_count_by_model (int ifindex, const char *pattern) +{ + mdns_finding **findings; + unsigned int findings_count = 0; + unsigned int i, answer; + ll_node *node; + + /* Collect matching devices */ + if (mdns_finding_list_count == 0) { + return 0; + } + + findings = alloca(sizeof(*findings) * mdns_finding_list_count); + + for (LL_FOR_EACH(node, &mdns_finding_list)) { + mdns_finding *mdns = OUTER_STRUCT(node, mdns_finding, node_list); + + if (mdns->finding.ifindex == ifindex && + mdns->finding.model != NULL && + fnmatch(pattern, mdns->finding.model, 0) == 0) { + + findings[findings_count] = mdns; + findings_count ++; + } + } + + /* Sort by ifindex + name, then count only distinct devices */ + qsort(findings, findings_count, sizeof(*findings), + zeroconf_finding_qsort_by_index_name); + + if (findings_count <= 1) { + return findings_count; + } + + answer = 1; + for (i = 1; i < findings_count; i ++) { + mdns_finding **p1 = &findings[i - 1]; + mdns_finding **p2 = &findings[i]; + + if (zeroconf_finding_qsort_by_index_name(p1, p2) != 0) { + answer ++; + } + } + + return answer; +} + /***** Initialization and cleanup *****/ /* Initialize MDNS diff --git a/airscan-wsdd.c b/airscan-wsdd.c index 82cc5d8..0b4883e 100644 --- a/airscan-wsdd.c +++ b/airscan-wsdd.c @@ -11,6 +11,7 @@ #include "airscan.h" #include +#include #include #include #include @@ -42,6 +43,7 @@ typedef struct { int fd; /* File descriptor */ int ifindex; /* Interface index */ + netif_name ifname; /* Interface name, for logging */ bool ipv6; /* We are on IPv6 */ eloop_fdpoll *fdpoll; /* Socket fdpoll */ eloop_timer *timer; /* Retransmit timer */ @@ -1177,12 +1179,39 @@ wsdd_resolver_read_callback (int fd, void *data, ELOOP_FDPOLL_MASK mask) } } +/* Count discovered devices with model names matching the specified parent. + * + * Pattern is the glob-style expression, applied to the model name + * of discovered devices. + */ +static unsigned int +wsdd_resolver_count_devices_by_model (wsdd_resolver *resolver, const char *pattern) +{ + unsigned int answer = 0; + ll_node *node; + wsdd_finding *wsdd; + + /* Lookup existent finding */ + for (LL_FOR_EACH(node, &wsdd_finding_list)) { + wsdd = OUTER_STRUCT(node, wsdd_finding, list_node); + if (wsdd->finding.ifindex == resolver->ifindex && + wsdd->finding.model != NULL && + wsdd->published && + fnmatch(pattern, wsdd->finding.model, 0) == 0) { + answer ++; + } + } + + return answer; +} + /* Retransmit timer callback */ static void wsdd_resolver_timer_callback (void *data) { wsdd_resolver *resolver = data; + const char pattern[] = "Pantum*"; resolver->timer = NULL; @@ -1190,9 +1219,25 @@ wsdd_resolver_timer_callback (void *data) * Should we use extended discovery time? */ if (resolver->time_elapsed >= resolver->time_limit && - resolver->time_limit < WSDD_DISCOVERY_TIME_EX && - zeroconf_device_exist_unpaired_mdns(resolver->ifindex, "Pantum*")) { - resolver->time_limit = WSDD_DISCOVERY_TIME_EX; + resolver->time_limit < WSDD_DISCOVERY_TIME_EX) { + unsigned int cnt_mdns, cnt_wsdd; + + cnt_mdns = mdns_device_count_by_model(resolver->ifindex, pattern); + cnt_wsdd = wsdd_resolver_count_devices_by_model(resolver, pattern); + + if (cnt_wsdd < cnt_mdns) { + const char *proto = resolver->ipv6 ? "ipv6" : "ipv4"; + + log_debug(wsdd_log, "%s@%s: \"%s\": MDNS/WSDD count: %d/%d", + proto, resolver->ifname.text, + pattern, cnt_mdns, cnt_wsdd); + + log_debug(wsdd_log, "%s@%s: extending discovery time (%d->%d ms)", + proto, resolver->ifname.text, + resolver->time_limit, WSDD_DISCOVERY_TIME_EX); + + resolver->time_limit = WSDD_DISCOVERY_TIME_EX; + } } /* Time limit not reached yet? */ @@ -1296,6 +1341,7 @@ wsdd_resolver_new (const netif_addr *addr, bool initscan) /* Build resolver structure */ resolver->ifindex = addr->ifindex; + resolver->ifname = addr->ifname; resolver->time_limit = WSDD_DISCOVERY_TIME; /* Open a socket */ diff --git a/airscan-zeroconf.c b/airscan-zeroconf.c index a685a1c..2095021 100644 --- a/airscan-zeroconf.c +++ b/airscan-zeroconf.c @@ -393,59 +393,6 @@ zeroconf_device_is_blacklisted (zeroconf_device *device) return NULL; } -/* Check if there are unpaired MDNS-only devices with model names - * matching the specified parent. - * - * WSDD uses this function to decide when to use extended discovery - * time (some devices are known to be hard for WD-Discovery) - * - * Pattern is the glob-style expression, applied to the model name - * of discovered devices. - */ -bool -zeroconf_device_exist_unpaired_mdns (int ifindex, const char *pattern) -{ - ll_node *node, *node2; - zeroconf_device *device; - zeroconf_finding *finding; - - for (LL_FOR_EACH(node, &zeroconf_device_list)) { - device = OUTER_STRUCT(node, zeroconf_device, node_list); - - /* If not the MDNS device or device already paired with the - * WSDD buddy -- skip the device - */ - if (!zeroconf_device_is_mdns(device) || device->buddy != NULL) { - continue; - } - - /* If model name doesn't match - skip the device - */ - if (device->model == NULL || - fnmatch(pattern, zeroconf_device_model(device), 0) != 0) { - continue; - } - - /* The last check: does the device seen on the specified interface? - */ - for (LL_FOR_EACH(node2, &device->findings)) { - finding = OUTER_STRUCT(node2, zeroconf_finding, list_node); - - if (finding->ifindex == ifindex) { - log_debug(zeroconf_log, - "Found unpaired MDNS device; WSDD discovery time extended"); - log_debug(zeroconf_log, - "Unpaired MDNS device: \"%s\" (interface %d)", - device->model, ifindex); - - return true; - } - } - } - - return false; -} - /******************** Merging devices *********************/ /* Recompute device->buddy for all devices */ @@ -880,6 +827,28 @@ zeroconf_find_static_by_ident (const char *ident) return NULL; } +/***** Miscellaneous functions for zeroconf_finding *****/ + +/* Compare two pointers to pointers to zeroconf_finding (zeroconf_finding**) + * by index+name, for qsort + */ +int +zeroconf_finding_qsort_by_index_name (const void *p1, const void *p2) +{ + const zeroconf_finding *f1 = *(const zeroconf_finding * const *) p1; + const zeroconf_finding *f2 = *(const zeroconf_finding * const *) p2; + + if (f1->ifindex < f2->ifindex) { + return -1; + } + + if (f1->ifindex > f2->ifindex) { + return 1; + } + + return strcmp(f1->name, f2->name); +} + /******************** Events from discovery providers *********************/ /* Publish the zeroconf_finding. */ diff --git a/airscan.h b/airscan.h index e8c89dd..c78e0c5 100644 --- a/airscan.h +++ b/airscan.h @@ -2852,6 +2852,12 @@ typedef struct { ll_node list_node; /* Node in device's list of findings */ } zeroconf_finding; +/* Compare two pointers to pointers to zeroconf_finding (zeroconf_finding**) + * by index+name, for qsort + */ +int +zeroconf_finding_qsort_by_index_name (const void *p1, const void *p2); + /* Publish the zeroconf_finding. * * Memory, referred by the finding, remains owned by @@ -2896,18 +2902,6 @@ zeroconf_init (void); void zeroconf_cleanup (void); -/* Check if there are unpaired MDNS-only devices with model names - * matching the specified parent. - * - * WSDD uses this function to decide when to use extended discovery - * time (some devices are known to be hard for WD-Discovery) - * - * Pattern is the glob-style expression, applied to the model name - * of discovered devices. - */ -bool -zeroconf_device_exist_unpaired_mdns (int ifindex, const char *pattern); - /* Get list of devices, in SANE format */ const SANE_Device** @@ -3067,6 +3061,21 @@ mdns_query_get_answer (const mdns_query *query); void* mdns_query_get_ptr (const mdns_query *query); +/* mdns_device_count_by_model returns count of distinct devices + * with model names matching the specified parent. + * + * Several instances of the same device (i.e. printer vs scanner) are + * counted only once per network interface. + * + * WSDD uses this function to decide when to use extended discovery + * time (some devices are known to be hard for WD-Discovery) + * + * Pattern is the glob-style expression, applied to the model name + * of discovered devices. + */ +unsigned int +mdns_device_count_by_model (int ifindex, const char *pattern); + /******************** WS-Discovery ********************/ /* Called by zeroconf to notify wsdd about initial scan timer expiration */