patch to support mDNS and multicast to servers #148

ka2ddo · 2021-01-06T21:10:21Z

changes so that multicast IP addresses can be used for the DNS "server", so that mDNS servers can be queried to support DNS-SD (Service Discovery).

add conditional logic to support receiving either a byte array of one answer (existing unicast server behavior), or an ArrayList containing one or more byte arrays (multiple responses to multicast query from multiple mDNS servers).

Add support to wait for multiple answers to arrive if and only if the address of the DNS server is a multicast address.

fix misleading text in logging message where the handling has changed.

ibauersachs · 2021-01-06T22:19:40Z

Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while.

In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times.

Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. sendrecv could just return a Future<List<byte[]>> where the single transaction always returns a list with a single entry. Distinguishing if answers should be combined or not can still be done based on the server's destination address.

ka2ddo · 2021-01-06T22:51:46Z

OK. I was trying to minimize changes to existing (unicast) logic so I wouldn't break anything. But I can rework it to reduce code redundancy. Andrew

…

________________________________________ From: Ingo Bauersachs <notifications@github.com> Sent: Wednesday, January 6, 2021 5:19 PM To: dnsjava/dnsjava Cc: Andrew Pavlin; Author Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148) Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while. In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times. Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. sendrecv could just return a Future<List<byte[]>> where the single transaction always returns a list with a single entry. Distinguishing if answers should be combined or not can still be done based on the server's destination address. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTJNLGDIR2OPIHKUNF3SYTOYZANCNFSM4VYCJ7VA>.

Refactor the mDNS changes to reduce duplicate code.

refactor most of the duplicated code for error-checking a received query response datagram into a method object.

ka2ddo · 2021-01-07T14:40:51Z

This is bizarre. The code compiles, but something about a format check in the source code in your maven script caused it to be rejected. And I copied the format that was already in those source files. So I looked at the error from the pull request build, so I am making a third change to my branch of the code base to keep the format-checking Maven plugin from griping.

…

________________________________________ From: Ingo Bauersachs <notifications@github.com> Sent: Wednesday, January 6, 2021 5:19 PM To: dnsjava/dnsjava Cc: Andrew Pavlin; Author Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148) Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while. In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times. Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. sendrecv could just return a Future<List<byte[]>> where the single transaction always returns a list with a single entry. Distinguishing if answers should be combined or not can still be done based on the server's destination address. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTJNLGDIR2OPIHKUNF3SYTOYZANCNFSM4VYCJ7VA>.

resolve maven com.coveo:format plugin's complaints

ibauersachs

Have you looked at some details of rfc6762? Specifically:

port 5353 must not be used as the local port as we're not fully compliant
Section 3 mandates that any query to .local. goes to an mDNS server. Not sure how to properly address that, enabling it by default is not an option because .local has been abused by non-mdns setups too much. But I could imagine that the ExtendedResolver sends .local to the multicast address if the multicast address is one of the resolver addresses. A multicast resolver address could be automatically added if a (new) property is set.
Setting the unicast-response bit should be easier to set/strip out (DClass). Not quite sure how repsonses look like. If the bit is still set in the response, it needs to be stripped after validation (or the answers won't be recognized as class IN), otherwise the validation needs to accept responses without the bit set as valid

What's an easy way to live test this code? Should having some Windows/Apple devices in the network suffice?

src/main/java/org/xbill/DNS/SimpleResolver.java

ibauersachs · 2021-02-04T21:25:19Z

src/main/java/org/xbill/DNS/SimpleResolver.java

+
+      // Check that the response is long enough.
+      if (in.length < Header.LENGTH) {
+        exc = new WireParseException("invalid DNS header - too short");


Why don't you just throw these exceptions? They're unsual enough that I wouldn't consider it to be normal code flow.

I didn't want to throw them unless there were no successful responses to a mDNS query. mDNS can get garbage packets back which should be silently ignored.And, because there could be multiple responses, I only wanted to throw the exception if no valid responses were returned (i.e., hacker returned garbage, then valid Avahi-daemon returned a legitimate answer [in that order], then the query shouldn't fail).

Yes, I get that, but instead of returning null and then checking the exception instance from a getter, you can throw, catch and still continue in the mDNS case.

ibauersachs · 2021-02-04T22:16:15Z

This is bizarre. The code compiles, but something about a format check in the source code in your maven script caused it to be rejected. And I copied the format that was already in those source files. So I looked at the error from the pull request build, so I am making a third change to my branch of the code base to keep the format-checking Maven plugin from griping.

You can auto-format the code by running the Maven target fmt:format. This applies most of the Google Java Format code style. If you're using IntelliJ, you can setup a plugin to format with Ctrl+Alt+L: https://github.com/google/google-java-format#intellij-android-studio-and-other-jetbrains-ides

ka2ddo · 2021-02-09T15:03:41Z

Re: reformatting: I didn't want to reformat any lines I didn't change, and cause spurious non-changing source code changes. Such behavior of IDEs (that "always know better than you do how your code should be formatted", especially when multiple developers' IDEs are configured differently) makes it a pain to review code changes, so I am always very careful not to change the styling of other lines of code (not even switching spaces versus tabs).

ka2ddo · 2021-02-09T15:29:19Z

Have you looked at some details of rfc6762? Specifically:

I sure did. That's how I came up with the idea of deliberately timing out the resolver so I could wait for multiple answers to come in.

port 5353 must not be used as the local port as we're not fully compliant

My proposed changes don't specify the port number. The caller is currently required to be smart enough to specify port 5353 when they also specify a multicast address as the "address" of the remote DNS server, since the SimpleResolver constructor I tested with takes InetSocketAddress objects that include the port number. I suppose the constructors taking a String or an InetAddress should check for multicast addresses and switch from DEFAULT_PORT (= 53) to 5353 automatically.

Or are you saying I need to add a safety check for when the local port is selected to ensure it doesn't accidentally get 5353 as the port number?

Section 3 mandates that any query to .local. goes to an mDNS server. Not sure how to properly
address that, enabling it by default is not an option because .local has been abused by non-mdns
setups too much. But I could imagine that the ExtendedResolver sends .local to the multicast address
if the multicast address is one of the resolver addresses. A multicast resolver address could be
automatically added if a (new) property is set.

That would have been a larger-impact change that I didn't need to do for my limited use-case, which knew that I was making a ".local" query. Would you like me to make the additional changes to automatically switch to multicast for such queries? Problem is, can I safely assume only IPv6 or only IPv4 for the default multicast address?

Setting the unicast-response bit should be easier to set/strip out (DClass). Not quite sure how
repsonses look like. If the bit is still set in the response, it needs to be stripped after validation (or
the answers won't be recognized as class IN), otherwise the validation needs to accept responses
without the bit set as valid

I didn't catch that, because my tests all worked correctly with multicast responses. I never received such a response when I did a multicast query in my tests. But I can make such a change to clear that bit in the received responses. Would you like me to do so?

What's an easy way to live test this code? Should having some Windows/Apple devices in the network suffice?

Have a Linux system on the LAN with the avahi-daemon running, then use the avahi-publish command to register some mDNS entries for testing. I suppose Apple devices on the LAN would do the same thing, but you would have less control over what answers you could get.

ibauersachs · 2021-02-10T19:36:04Z

Or are you saying I need to add a safety check for when the local port is selected to ensure it doesn't accidentally get 5353 as the port number?

Yes, that's what I meant.

Section 3 mandates that any query to .local. goes to an mDNS server. Not sure how to properly
address that, enabling it by default is not an option because .local has been abused by non-mdns
setups too much. But I could imagine that the ExtendedResolver sends .local to the multicast address
if the multicast address is one of the resolver addresses. A multicast resolver address could be
automatically added if a (new) property is set.

That would have been a larger-impact change that I didn't need to do for my limited use-case, which knew that I was making a ".local" query. Would you like me to make the additional changes to automatically switch to multicast for such queries?

I'm not sure how useful it is. I guess we could always add it later if it seems worthwhile.

Problem is, can I safely assume only IPv6 or only IPv4 for the default multicast address?

It depends :-)
If would only activate mDNS if the Resolver contains a broadcast address, and that address would already dictate which IP version to use. If both an IPv4 and an IPv6 address is present, then I guess both should be queried.

Setting the unicast-response bit should be easier to set/strip out (DClass). Not quite sure how
repsonses look like. If the bit is still set in the response, it needs to be stripped after validation (or
the answers won't be recognized as class IN), otherwise the validation needs to accept responses
without the bit set as valid

I didn't catch that, because my tests all worked correctly with multicast responses. I never received such a response when I did a multicast query in my tests. But I can make such a change to clear that bit in the received responses. Would you like me to do so?

I'm not sure it's required, and I couldn't conclude from the RFC without looking at packets.

What's an easy way to live test this code? Should having some Windows/Apple devices in the network suffice?

Have a Linux system on the LAN with the avahi-daemon running, then use the avahi-publish command to register some mDNS entries for testing. I suppose Apple devices on the LAN would do the same thing, but you would have less control over what answers you could get.

Okay, I'll look at some of those packets, then I'll know if that bit mentioned above needs attention or not. Thanks.

ibauersachs · 2021-02-10T21:16:55Z

Okay, so I sent some sample queries on my LAN with the unicode flag masked in, like this:

public class Test {
  public static void main(String[] args) throws IOException {
    SimpleResolver sr = new SimpleResolver(new InetSocketAddress("224.0.0.251", 5353));
    Record q = Record.newRecord(Name.fromConstantString("macmini.local."), Type.A, DClass.IN | 0x8000);
    Message mq = Message.newQuery(q);
    System.out.println(sr.send(mq));
  }
}

I get a unicast reply from the Mac Mini, but also two multicast responses from avahi-daemons I seem to have running. The answers from the avahi-daemons are thrown away because the validation in verify1response doesn't take the unicast flag into account (or rather: doesn't ignore it).

The relevant part from the RFC is this sentence (emphasis mine):

When this bit is set in a
question, it indicates that the querier is willing to accept unicast
replies in response to this specific query, as well as the usual
multicast responses. These questions requesting unicast responses
are referred to as "QU" questions, to distinguish them from the more
usual questions requesting multicast responses ("QM" questions).

Setting the unicast flag in the first hand with masking 0x8000 into DClass.IN is unnatural. There needs to be a better way to set this flag. Parsing and removing needs to be better as well because a printed messages will look like this:

;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 56018
;; flags: qr aa ; qd: 1 an: 1 au: 0 ad: 0 
;; QUESTIONS:
;;	macmini.local., type = A, class = CLASS32769

;; ANSWERS:
macmini.local.		10	IN	A	192.168.10.103

;; AUTHORITY RECORDS:

;; ADDITIONAL RECORDS:

;; Message size: 47 bytes

Note CLASS32769 instead of IN, and maybe a unicast indication. Wireshark for example shows such a query as class IN, "QU" question or as class IN, "QM" question.
Answer records also have a special meaning of the highest bit in the class, meaning "Cache flush" if it is set (sections 10.3 and 10.4).

I'm not sure how to best handle this flag (in questions and answers). Maybe @nresare could chime in here and has an idea?

of TCP vs. UDP DNS queries and preventing accidental collision of UDP client port with mDNS server.

ka2ddo · 2021-03-13T21:46:00Z

additional changes you requested submitted to mDNS_patch_ka2ddo branch.

ka2ddo · 2021-03-13T21:52:16Z

and fixing the formatting again....

first build failure.

ka2ddo · 2021-03-13T22:01:28Z

and again making the formatter happy (since it only makes one complaint at a time).

ibauersachs

Thanks for the update. I've left a bunch of comments inline.

The bigger question about how to handle the DClass in questions and responses however is still not clear:

In questions, the highest bit set in the Class means Unicast
In answers, the highest bit set in the Class means "Cache flush"

For both cases, this needs to be properly handled, i.e. not only in the SimpleResolver but also in e.g. Message.toString(), Record.getDClass() and probably lots of other locations. Maybe a flag boolean isMulticast; in the Message and Record classes is an option.

and again making the formatter happy (since it only makes one complaint at a time).

You can run the checks locally with mvn verify or make the checks happy by applying the formats to your IDE with the links provided previously.

ibauersachs · 2021-03-14T10:26:49Z

src/main/java/org/xbill/DNS/DClass.java

@@ -31,6 +31,9 @@
  /** Matches any class */
  public static final int ANY = 255;

+  /** Indicates on mDNS that querier will accept unicast replies from a multicast request. */
+  public static final int UNICAST_RESPONSE = 0x8000;


-> UNICAST_RESPONSE_FLAG

OK, in next request.

ibauersachs · 2021-03-14T10:29:36Z

src/main/java/org/xbill/DNS/NioUdpClient.java

@@ -169,6 +229,9 @@ private void silentCloseChannel() {

              addr = new InetSocketAddress(local.getAddress(), port);
            }
+            if (addr.getPort() == SimpleResolver.RESERVED_MDNS_PORT) {


addr can be null if the config option dnsjava.udp.ephemeral.use_ephemeral_port is set. Also, port 5353 is a valid source-port in normal DNS

I kind of disagree with the statement that "port 5353 is valid in normal DNS". Since mDNS is becoming more prevalent, it would be recommended not to use that port accidentally to avoid receiving requests from other hosts when you're expecting replies. And it doesn't hurt to avoid it, since this part of the code no longer differentiates between receiving DNS and mDNS replies.

In regular DNS the socket is bound, so the only replies accepted are from a DNS server. The case shouldn't occur when using defaults though.

But addr==null needs to be handled anyway.

ibauersachs · 2021-03-14T10:29:56Z

src/main/java/org/xbill/DNS/SimpleResolver.java

@@ -28,6 +29,9 @@
  /** The default port to send queries to */
  public static final int DEFAULT_PORT = 53;

+  /** The port we can't use as a client because it's reserved for mDNS servers. */
+  public static final int RESERVED_MDNS_PORT = 5353;


-> MDNS_PORT

ibauersachs · 2021-03-14T10:31:05Z

src/main/java/org/xbill/DNS/SimpleResolver.java

-                        + Type.string(response.getQuestion().getType())));
-            return f;
+          VerifyOneResponse verifyOneResponse = new VerifyOneResponse(query, qid);
+          @SuppressWarnings("unchecked")


This is no longer necessary

ibauersachs · 2021-03-14T10:31:55Z

src/main/java/org/xbill/DNS/SimpleResolver.java

    if (tcp) {
      result = NioTcpClient.sendrecv(localAddress, address, query, out, timeoutValue);
    } else {
      result = NioUdpClient.sendrecv(localAddress, address, out, udpSize, timeoutValue);
    }

    return result.thenComposeAsync(
-        in -> {
+        v -> {


name this something useful, e.g. answers (v is typically used for Void, which is not the case here)

OK, in next request.

ibauersachs · 2021-03-14T14:45:25Z

src/main/java/org/xbill/DNS/NioUdpClient.java

+        source = channel.receive(buffer);
+        read = buffer.position();
+        if (read <= 0 || source == null) {
+          return; // ignore this datagram


This is a lot of code duplication. The return here vs. EOFException could be:

if (remoteSocketAddress.getAddress().isMulticast()) { return; } throw new EOFException();

ibauersachs · 2021-03-14T14:57:59Z

src/main/java/org/xbill/DNS/NioUdpClient.java

+      byte[] data = new byte[read];
+      System.arraycopy(buffer.array(), 0, data, 0, read);
+      verboseLog("UDP read", channel.socket().getLocalSocketAddress(), source, data);
+      answers.add(data);


This is the only other line in this method that differs for mDNS. Instead of duplicating everything, move the processing of the data byte-array into a separate method, e.g.

// in Transaction: ... System.arraycopy(buffer.array(), 0, data, 0, read); verboseLog("UDP read", channel.socket().getLocalSocketAddress(), source, data); processAnswer(data); } processAnswer(byte[] data) { silentCloseChannel(); f.complete(Collections.singletonList(data)); pendingTransactions.remove(this); } // in MultiAnswerTransaction: processAnswer(byte[] data) { answers.add(data); }

ibauersachs · 2021-03-14T15:04:59Z

src/main/java/org/xbill/DNS/NioUdpClient.java

+    private ArrayList<byte[]> answers = new ArrayList<>();
+
+    @Override
+    void closeTransaction() {


closeTransaction is only triggered when the timeout has expired. Is this really the only condition on which you want to return answers? What about the case where one is only interested in the first answer, as fast as possible?

The timeout processing currently only runs every second. Is that still sufficient?

ibauersachs · 2021-03-14T15:14:03Z

src/main/java/org/xbill/DNS/SimpleResolver.java

@@ -444,4 +426,90 @@ private Message sendAXFR(Message query) throws IOException {
  public String toString() {
    return "SimpleResolver [" + address + "]";
  }
+
+  private class VerifyOneResponse {


Actually, this doesn't need to be a class. As I mentioned before, throwing the exceptions is fine and whole thing can be a method: Message verifyResponse(Message query, byte[] in) throws WireParseException { ... }

ibauersachs · 2021-03-14T15:18:30Z

src/main/java/org/xbill/DNS/SimpleResolver.java

+        return null;
+      }
+
+      if ((query.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE)


This masking must only be applied for mDNS, not regular DNS

In the mDNS case, the mask should be 0x7FFF so that 32768 to 65535 is ignored (see the note at IANA or in the RFC)

ka2ddo · 2021-07-27T00:35:44Z

Hello. Sorry I'm behind on this patch. I hope to get back to it next week. Andrew

…

________________________________________ From: cary-xian ***@***.***> Sent: Monday, July 26, 2021 8:26 PM To: dnsjava/dnsjava Cc: Andrew Pavlin; Author Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148) dns — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTKTWMZGUGRIXCGUBK3TZX4JVANCNFSM4VYCJ7VA>.

arnocornette · 2022-09-15T23:24:17Z

Is this still a WIP? And are you still working on this @ka2ddo? I wouldn't mind giving it a look.

ka2ddo added 3 commits January 6, 2021 15:46

Update SimpleResolver.java

504fe0d

add conditional logic to support receiving either a byte array of one answer (existing unicast server behavior), or an ArrayList containing one or more byte arrays (multiple responses to multicast query from multiple mDNS servers).

Update NioUdpClient.java

fce0cf8

Add support to wait for multiple answers to arrive if and only if the address of the DNS server is a multicast address.

Update SimpleResolver.java

544182b

fix misleading text in logging message where the handling has changed.

ibauersachs linked an issue Jan 6, 2021 that may be closed by this pull request

Add support for mDNS by allowing multicast server address #147

Open

ka2ddo added 2 commits January 7, 2021 09:16

reduce duplicate code in mDNS support changes

855b7be

Refactor the mDNS changes to reduce duplicate code.

reduce duplicated code in mDNS changes

1cf8fb8

refactor most of the duplicated code for error-checking a received query response datagram into a method object.

ka2ddo added 2 commits January 7, 2021 09:44

resolve format complaints in mDNS-modified code

cc916e5

resolve maven com.coveo:format plugin's complaints

resolve format complaints in mDNS changes

e838df6

ibauersachs requested changes Feb 4, 2021

View reviewed changes

ka2ddo added 2 commits March 13, 2021 16:19

resolve issues reported by dnsjava over regarding consistent behavior

92d98d5

of TCP vs. UDP DNS queries and preventing accidental collision of UDP client port with mDNS server.

a few more bugfixes to the mDNS changes per dnsjava author's request.

80aa3cd

make the format checker happy (again!).

31f2621

more formatting issues that the online checker didn't report on the

c1ce27b

first build failure.

ibauersachs requested changes Mar 14, 2021

View reviewed changes

ibauersachs force-pushed the master branch from 07cedfa to 9a31769 Compare July 30, 2021 13:49

ibauersachs force-pushed the master branch from e6083ea to 7cef3e2 Compare August 7, 2021 18:20

ibauersachs force-pushed the master branch from 2f8e897 to a0fafcb Compare August 27, 2023 18:48

ibauersachs force-pushed the master branch from a0fafcb to 30ceab2 Compare August 27, 2023 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch to support mDNS and multicast to servers #148

patch to support mDNS and multicast to servers #148

ka2ddo commented Jan 6, 2021

ibauersachs commented Jan 6, 2021

ka2ddo commented Jan 6, 2021 via email

ka2ddo commented Jan 7, 2021 via email

ibauersachs left a comment

ibauersachs Feb 4, 2021

ka2ddo Feb 9, 2021

ibauersachs Feb 10, 2021

ibauersachs commented Feb 4, 2021

ka2ddo commented Feb 9, 2021 •

edited

ka2ddo commented Feb 9, 2021

ibauersachs commented Feb 10, 2021

ibauersachs commented Feb 10, 2021

ka2ddo commented Mar 13, 2021

ka2ddo commented Mar 13, 2021

ka2ddo commented Mar 13, 2021

ibauersachs left a comment

ibauersachs Mar 14, 2021

ka2ddo Mar 15, 2021

ibauersachs Mar 14, 2021

ka2ddo Mar 15, 2021

ibauersachs Mar 15, 2021

ibauersachs Mar 14, 2021

ka2ddo Mar 15, 2021

ibauersachs Mar 14, 2021

ka2ddo Mar 15, 2021

ibauersachs Mar 14, 2021

ka2ddo Mar 15, 2021

ibauersachs Mar 14, 2021

ibauersachs Mar 14, 2021

ibauersachs Mar 14, 2021

ibauersachs Mar 14, 2021

ibauersachs Mar 14, 2021

ka2ddo commented Jul 27, 2021 via email

arnocornette commented Sep 15, 2022 •

edited

patch to support mDNS and multicast to servers #148

Are you sure you want to change the base?

patch to support mDNS and multicast to servers #148

Conversation

ka2ddo commented Jan 6, 2021

ibauersachs commented Jan 6, 2021

ka2ddo commented Jan 6, 2021 via email

ka2ddo commented Jan 7, 2021 via email

ibauersachs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibauersachs commented Feb 4, 2021

ka2ddo commented Feb 9, 2021 • edited

ka2ddo commented Feb 9, 2021

ibauersachs commented Feb 10, 2021

ibauersachs commented Feb 10, 2021

ka2ddo commented Mar 13, 2021

ka2ddo commented Mar 13, 2021

ka2ddo commented Mar 13, 2021

ibauersachs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ka2ddo commented Jul 27, 2021 via email

arnocornette commented Sep 15, 2022 • edited

ka2ddo commented Feb 9, 2021 •

edited

arnocornette commented Sep 15, 2022 •

edited