Fatal error if no udp threads but using quic

[mattyw]

Fatal error if no udp threads but using quic

If proxy.config.http.server_ports contains an entry to allow quic
connections on a given port but there no udp threads ATS used to
silently ignore these connections.

Given that this configuration isn't valid we raise a fatal error.

We also raise a fatal error if quic is listed in server ports but ATS
was built without quic support.

[mattyw]

255 chosen arbitrarily - would appreciate guidance of a better number

[brbzull0]

Not sure about this, but there is a HttpProxyPort::Group which gets filled in and have the information you need.

[maskit]

This check could be done as a part of this block, if we want to have a specialized message for QUIC (the block already has code for unrecognized options).

trafficserver/src/records/RecHttp.cc

Lines 367 to 463 in fc5918e

	for (auto item : values) {
	if (isdigit(item[0])) { // leading digit -> port value
	char *ptr;
	int port = strtoul(item, &ptr, 10);
	if (ptr == item) {
	// really, this shouldn't happen, since we checked for a leading digit.
	Warning("Mangled port value '%s' in port configuration '%s'", item, opts);
	} else if (port <= 0 || 65536 <= port) {
	Warning("Port value '%s' out of range (1..65535) in port configuration '%s'", item, opts);
	} else {
	m_port = port;
	zret = true;
	}
	} else if (nullptr != (value = this->checkPrefix(item, OPT_FD_PREFIX, OPT_FD_PREFIX_LEN))) {
	char *ptr; // tmp for syntax check.
	int fd = strtoul(value, &ptr, 10);
	if (ptr == value) {
	Warning("Mangled file descriptor value '%s' in port descriptor '%s'", item, opts);
	} else {
	m_fd = fd;
	zret = true;
	}
	} else if (nullptr != (value = this->checkPrefix(item, OPT_INBOUND_IP_PREFIX, OPT_INBOUND_IP_PREFIX_LEN))) {
	if (0 == ip.load(value)) {
	m_inbound_ip = ip;
	} else {
	Warning("Invalid IP address value '%s' in port descriptor '%s'", item, opts);
	}
	} else if (nullptr != (value = this->checkPrefix(item, OPT_OUTBOUND_IP_PREFIX, OPT_OUTBOUND_IP_PREFIX_LEN))) {
	if (swoc::IPAddr addr; addr.load(value)) {
	this->m_outbound = addr;
	} else {
	Warning("Invalid IP address value '%s' in port descriptor '%s'", item, opts);
	}
	} else if (0 == strcasecmp(OPT_COMPRESSED, item)) {
	m_type = TRANSPORT_COMPRESSED;
	} else if (0 == strcasecmp(OPT_BLIND_TUNNEL, item)) {
	m_type = TRANSPORT_BLIND_TUNNEL;
	} else if (0 == strcasecmp(OPT_IPV6, item)) {
	m_family = AF_INET6;
	af_set_p = true;
	} else if (0 == strcasecmp(OPT_IPV4, item)) {
	m_family = AF_INET;
	af_set_p = true;
	} else if (0 == strcasecmp(OPT_SSL, item)) {
	m_type = TRANSPORT_SSL;
	#if TS_USE_QUIC == 1
	} else if (0 == strcasecmp(OPT_QUIC, item)) {
	m_type = TRANSPORT_QUIC;
	#endif
	} else if (0 == strcasecmp(OPT_PLUGIN, item)) {
	m_type = TRANSPORT_PLUGIN;
	} else if (0 == strcasecmp(OPT_PROXY_PROTO, item)) {
	m_proxy_protocol = true;
	} else if (0 == strcasecmp(OPT_TRANSPARENT_INBOUND, item)) {
	#if TS_USE_TPROXY
	m_inbound_transparent_p = true;
	#else
	Warning("Transparency requested [%s] in port descriptor '%s' but TPROXY was not configured.", item, opts);
	#endif
	} else if (0 == strcasecmp(OPT_TRANSPARENT_OUTBOUND, item)) {
	#if TS_USE_TPROXY
	m_outbound_transparent_p = true;
	#else
	Warning("Transparency requested [%s] in port descriptor '%s' but TPROXY was not configured.", item, opts);
	#endif
	} else if (0 == strcasecmp(OPT_TRANSPARENT_FULL, item)) {
	#if TS_USE_TPROXY
	m_inbound_transparent_p = true;
	m_outbound_transparent_p = true;
	#else
	Warning("Transparency requested [%s] in port descriptor '%s' but TPROXY was not configured.", item, opts);
	#endif
	} else if (0 == strcasecmp(OPT_TRANSPARENT_PASSTHROUGH, item)) {
	#if TS_USE_TPROXY
	m_transparent_passthrough = true;
	#else
	Warning("Transparent pass-through requested [%s] in port descriptor '%s' but TPROXY was not configured.", item, opts);
	#endif
	} else if (0 == strcasecmp(OPT_ALLOW_PLAIN, item)) {
	m_allow_plain = true;
	} else if (0 == strcasecmp(OPT_MPTCP, item)) {
	if (mptcp_supported()) {
	m_mptcp = true;
	} else {
	Warning("Multipath TCP requested [%s] in port descriptor '%s' but it is not supported by this host.", item, opts);
	}
	} else if (nullptr != (value = this->checkPrefix(item, OPT_HOST_RES_PREFIX, OPT_HOST_RES_PREFIX_LEN))) {
	this->processFamilyPreference(value);
	host_res_set_p = true;
	} else if (nullptr != (value = this->checkPrefix(item, OPT_PROTO_PREFIX, OPT_PROTO_PREFIX_LEN))) {
	this->processSessionProtocolPreference(value);
	sp_set_p = true;
	} else {
	Warning("Invalid option '%s' in proxy port descriptor '%s'", item, opts);
	}
	}

[mattyw]

@maskit Yeah good suggestion! I tried looking into the records code to see a good place to put this logic in but couldn't find a good place. I see if I can make progress with your suggestion here.

[maskit]

I'm not sure if we should use Fatal. Even port configuration error is just a warning. It depends on the use case for sure, but QUIC being unavailable is not the end of the world.

[mattyw]

@maskit I think you might be right w.r.t warning - I bounced between fatal and warning a few times before proposing.

but QUIC being unavailable is not the end of the world.

Well, QUIC being unavailable when you've specifically asked ATS to serve quic could be considered to be the end of the world - it would certainly be surprising enough that I'd think we'd want to catch it sooner rather than later

[maskit]

How you catch misconfiguration and how ATS behave while configuration may be wrong are different things. You could watch for a warning to catch misconfiguration, or you could also watch for the UDP ports. Ideally you should periodically send health check requests. Look at the code I mentioned above. You can try to enable mptcp where mptcp is unavailable on the host and that does not stop ATS. Lack of QUIC does not threaten the safety. QUIC can be unavailable even with proper configuration if middle boxes block it. I still think Fatal is too much.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.