Request unicast responses for resolving nodes when using minmdns (#11635

)

* Request unicast responses for resolving nodes when using minmdns

* Set unicast answer to true in browse nodes as well

* Make unit tests use platform mdns: chip tool and all clusters all need to be mdns servers (because tv  commisioning) so having a single central point seems more correct than having several mdns servers

* Set the chip_mdns value as a string

* Escape quotes for platform mdns  build

* Revert platform mdns usage in tests: avahi does not properly work for testing (tried locally, could not get it to work but minimal can work, need to figure out how)

* Add support for using netns namespaces for unit test runs - check to see  if that makes linux unit tests happy

* Do not restyle test_suites.sh

* Some script cleanup

* Temporarely run the tests on ipv4 only until I figure out ipv6 multicast support

* Cleanup netns as a final step, just in case

* Move cleanup again - cleanup is added again post-netns setup

* Code review comments, enable ipv6 back

* Add note on why we stop restuling testsuites.sh

* Use unshare instead of sudo to run unit tests in namespaces

* Update remount logic to only apply for unshare environments

* Updated messaging logic

* Added a sleep for IPv6 and a comment as to why

.github/workflows/tests.yaml

@@ -86,11 +86,11 @@ jobs:
             - name: Run Tests
               timeout-minutes: 20
               run: |
-                  scripts/tests/test_suites.sh
+                  scripts/tests/test_suites.sh -n
             - name: Run TV Tests
               timeout-minutes: 10
               run: |
-                  scripts/tests/test_suites.sh  -a tv
+                  scripts/tests/test_suites.sh -n -a tv
             - name: Uploading core files
               uses: actions/upload-artifact@v2
               if: ${{ failure() }} && ${{ !env.ACT }}

.restyled.yaml

@@ -67,6 +67,7 @@ exclude:
     - "third_party/nanopb/repo/**/*"
     - "src/android/CHIPTool/gradlew" # gradle wrapper generated file
     - "third_party/android_deps/gradlew" # gradle wrapper generated file
+    - "scripts/tests/test_suites.sh" # overly agressive shell harden
 
 
 changed_paths:

scripts/tests/test_suites.sh

@@ -23,10 +23,15 @@ set -e
 # us tends to be 'tee').
 set -o pipefail
 
+declare INPUT_ARGS=$*
+
 declare -i iterations=2
 declare -i delay=0
 declare -i node_id=0x12344321
 declare -i background_pid=0
+declare -i use_netns=0
+declare -i root_remount=0
+declare -i clean_netns=0
 declare test_case_wrapper=()
 
 usage() {
@@ -38,22 +43,121 @@ usage() {
     echo "  -s CASE_NAME: runs single test case name (e.g. Test_TC_OO_2_2"
     echo "                for Test_TC_OO_2_2.yaml) (by default, all are run)"
     echo "  -w COMMAND: prefix all instantiations with a command (e.g. valgrind) (default: '')"
+    echo "  -n Use linux netns to isolate app and tool executables"
+    echo "  -c execute a netns cleanup and exit"
+    echo "  -r Execute a remount (INTERNAL USE for netns)"
     echo ""
     exit 0
 }
 
-# read shell arguments
-while getopts a:d:i:hs:w: flag; do
+declare app_run_prefix=""
+declare tool_run_prefix=""
+
+netns_setup() {
+    # 2 virtual hosts: for app and for the tool
+    ip netns add app
+    ip netns add tool
+
+    # create links for switch to net connections
+    ip link add eth-app type veth peer name eth-app-switch
+    ip link add eth-tool type veth peer name eth-tool-switch
+
+    # link the connections together
+    ip link set eth-app netns app
+    ip link set eth-tool netns tool
+
+    ip link add name br1 type bridge
+    ip link set br1 up
+    ip link set eth-app-switch master br1
+    ip link set eth-tool-switch master br1
+
+    # mark connections up
+    ip netns exec app ip addr add 10.10.10.1/24 dev eth-app
+    ip netns exec app ip link set dev eth-app up
+    ip netns exec app ip link set dev lo up
+    ip link set dev eth-app-switch up
+
+    ip netns exec tool ip addr add 10.10.10.2/24 dev eth-tool
+    ip netns exec tool ip link set dev eth-tool up
+    ip netns exec tool ip link set dev lo up
+    ip link set dev eth-tool-switch up
+
+    # Force IPv6 to use ULAs that we control
+    ip netns exec tool ip -6 addr flush eth-tool
+    ip netns exec app ip -6 addr flush eth-app
+    ip netns exec tool ip -6 a add fd00:0:1:1::2/64 dev eth-tool
+    ip netns exec app ip -6 a add fd00:0:1:1::3/64 dev eth-app
+
+    # TODO(andy314): IPv6 does Duplicate Address Detection even though
+    # we know these addresses are isolated. For a while IPv6 addresses
+    # will be in 'transitional' state and cannot be used.
+    #
+    # This sleep waits for the addresses to become 'global'. Ideally
+    # we should loop/wait here instead.
+    sleep 2
+}
+
+netns_cleanup() {
+    ip netns del app || true
+    ip netns del tool || true
+    ip link del br1 || true
+
+    # attempt  to delete orphaned items just in case
+    ip link del eth-tool || true
+    ip link del eth-tool-switch || true
+    ip link del eth-app || true
+    ip link del eth-app-switch || true
+}
+
+while getopts a:d:i:hs:w:ncr flag; do
     case "$flag" in
-        a) application=$OPTARG ;;
-        d) delay=$OPTARG ;;
-        h) usage ;;
-        i) iterations=$OPTARG ;;
-        s) single_case=$OPTARG ;;
-        w) test_case_wrapper=("$OPTARG") ;;
+    a) application=$OPTARG ;;
+    d) delay=$OPTARG ;;
+    h) usage ;;
+    i) iterations=$OPTARG ;;
+    s) single_case=$OPTARG ;;
+    w) test_case_wrapper=("$OPTARG") ;;
+    n) use_netns=1 ;;
+    c) clean_netns=1 ;;
+    r) root_remount=1 ;;
     esac
 done
 
+if [[ $clean_netns != 0 ]]; then
+    echo "Cleaning network namespaces"
+    netns_cleanup
+    exit 0
+fi
+
+if [[ $root_remount != 0 ]]; then
+    echo 'Creating a separate mount'
+    mount --make-private /
+    mount -t tmpfs tmpfs /run
+fi
+
+if [[ $use_netns != 0 ]]; then
+    echo "Using network namespaces"
+
+    if [[ `id -u` -ne 0 ]]; then
+        echo 'Executing in a new namespace: ' $0 -r $INPUT_ARGS
+        unshare --map-root-user -n -m $0 -r $INPUT_ARGS
+        exit 0
+    else
+        if [[ $root_remount -eq 0 ]]; then
+          # Running as root may be fine in docker/vm however this is not advised
+          # on workstations as changes are global and harder to undo
+          echo 'Running as root: this changes global network namespaces, not ideal'
+        fi
+    fi
+
+    netns_setup
+
+    app_run_prefix="ip netns exec app"
+    tool_run_prefix="ip netns exec tool"
+
+    trap netns_cleanup EXIT
+fi
+
 if [[ $application == "tv" ]]; then
     declare test_filenames="${single_case-TV_*}.yaml"
     cp examples/tv-app/linux/include/endpoint-configuration/chip_tv_config.ini /tmp/chip_tv_config.ini
@@ -77,6 +181,10 @@ cleanup() {
         # In case we died on a failure before we cleaned up our background task.
         kill -9 "$background_pid" || true
     fi
+
+    if [[ $use_netns != 0 ]]; then
+        netns_cleanup
+    fi
 }
 trap cleanup EXIT
 
@@ -123,7 +231,7 @@ for j in "${iter_array[@]}"; do
         touch "$chip_tool_log_file"
         rm -rf /tmp/pid
         (
-            stdbuf -o0 "${test_case_wrapper[@]}" out/debug/standalone/chip-"$application"-app &
+            ${app_run_prefix} stdbuf -o0 "${test_case_wrapper[@]}" out/debug/standalone/chip-"$application"-app &
             echo $! >&3
         ) 3>/tmp/pid | tee "$application_log_file" &
         while ! grep -q "Server Listening" "$application_log_file"; do
@@ -141,9 +249,9 @@ for j in "${iter_array[@]}"; do
             cat <(timeout 1 dns-sd -B _matterc._udp)
         fi
         echo "          * Pairing to device"
-        "${test_case_wrapper[@]}" out/debug/standalone/chip-tool pairing qrcode "$node_id" MT:D8XA0CQM00KA0648G00 | tee "$pairing_log_file"
+        ${tool_run_prefix} "${test_case_wrapper[@]}" out/debug/standalone/chip-tool pairing qrcode "$node_id" MT:D8XA0CQM00KA0648G00 | tee "$pairing_log_file"
         echo "          * Starting test run: $i"
-        "${test_case_wrapper[@]}" out/debug/standalone/chip-tool tests "$i" "$node_id" "$delay" | tee "$chip_tool_log_file"
+        ${tool_run_prefix} "${test_case_wrapper[@]}" out/debug/standalone/chip-tool tests "$i" "$node_id" "$delay" | tee "$chip_tool_log_file"
         # Prevent cleanup trying to kill a process we already killed.
         temp_background_pid=$background_pid
         background_pid=0

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -427,8 +427,7 @@ CHIP_ERROR MinMdnsResolver::SendQuery(mdns::Minimal::FullQName qname, mdns::Mini
 
     mdns::Minimal::Query query(qname);
     query.SetType(type).SetClass(mdns::Minimal::QClass::IN);
-    // TODO(cecille): Not sure why unicast response isn't working - fix.
-    query.SetAnswerViaUnicast(false);
+    query.SetAnswerViaUnicast(true);
 
     builder.AddQuery(query);
 
@@ -555,9 +554,9 @@ CHIP_ERROR MinMdnsResolver::SendPendingResolveQueries()
             Query query(instanceQName);
 
             query
-                .SetClass(QClass::IN)       //
-                .SetType(QType::ANY)        //
-                .SetAnswerViaUnicast(false) //
+                .SetClass(QClass::IN)      //
+                .SetType(QType::ANY)       //
+                .SetAnswerViaUnicast(true) //
                 ;
 
             // NOTE: type above is NOT A or AAAA because the name searched for is