23.05 dnsmasq, ipsets and mwan3 incompatibility?

OK, so just so I'm clear. Your specific concern is that either a user or a package will create a uci ipset definition, and since the dnsmasq nftset syntax is different than the ipset syntax (nftset requires table name, etc.) it will generate an invalid ipset option in /var/etc/dnsmasq.conf.cfg. Is that correct?

I suppose that might be true, but again, I'm not asking for uci ipset configs and mwan3 doesn't create uci ipsets at all. I'm asking if dnsmasq, compiled with -DHAVE_IPSET can read hand-written ipset options from /etc/dnsmasq.conf, and it seems like that does indeed work?

There is the possibility of name collisions between uci ipsets and /etc/dnsmasq.conf, but that possibility has always existed and is not new with 23.05.

1 Like

No, you are missing the point entirely.

Tl;DR: You cannot use fw4 to configure a proper iptables ipset for mwan3 to use, no matter how you compile dnsmasq.

Ipset is a separate package developed as an extension to iptables to allow a (often dynamic) database of ip addresses to be used by iptables. It knows nothing about nftables.
Fw4 cannot configure anything to do with iptables. It is entirely concened with nftables.

Nftsets is built in functionality of nftables to handle dynamic "sets" of data. One possible data type is a list of ip addresses.

Starting after 22.03, fw4 uses the term "ipset" to mean "nftset", and creates an nftables nftset for the support of a "set of ip addresses". Unfortunately it goes on to configure dnsmasq to translate some unique uci options that are very similar to the old iptables syntax, into nftables format.

This works for the Fw4 firewall settings (100% nftables), but fails entirely if another package (eg mwan3) needs a proper iptables ipset (using the ipset package).

Because of the forced translation of fw4 syntax into nftset format, adding ipset support to dnsmasq as well as nftset support will not work (and generally crashes dnsmasq).

If you turn off nftset and turn on ipset, then yes you can make it work by editing the config manually and using the ipset utility to create the database, but then fw4 will fail to configure its "ipsets" which are actually nftsets.

1 Like

I built the dnsmasq-full package with both options enabled, and it works, at least for the usecase I described (custom config in /etc/dnsmasq.conf).

> grep DISTRIB_RELEASE /etc/openwrt_release
DISTRIB_RELEASE='23.05.0'

> opkg install ipset
> opkg remove dnsmasq
> opkg install /tmp/dnsmasq-full_2.89-4_x86_64.ipk
...

> /usr/sbin/dnsmasq -v
Dnsmasq version 2.89  Copyright (c) 2000-2022 Simon Kelley
Compile time options: IPv6 GNU-getopt no-DBus UBus no-i18n no-IDN DHCP DHCPv6 no-Lua TFTP conntrack ipset nftset auth cryptohash DNSSEC no-ID loop-detect inotify dumpfile

> ipset create test-dnsmasq-population hash:ip
> ipset list test-dnsmasq-population
Name: test-dnsmasq-population
Type: hash:ip
Revision: 5
Header: family inet hashsize 1024 maxelem 65536 bucketsize 12 initval 0xfa751cde
Size in memory: 200
References: 0
Number of entries: 0
Members:

> echo "ipset=/www.google.com/test-dnsmasq-population" > /etc/dnsmasq.conf
> /etc/init.d/dnsmasq restart

> nslookup www.google.com 127.0.0.1
Server:		127.0.0.1
Address:	127.0.0.1:53

Non-authoritative answer:
Name:	www.google.com
Address: 142.251.46.164

Non-authoritative answer:
Name:	www.google.com
Address: 2607:f8b0:4005:80f::2004

> ipset list test-dnsmasq-population
Name: test-dnsmasq-population
Type: hash:ip
Revision: 5
Header: family inet hashsize 1024 maxelem 65536 bucketsize 12 initval 0xfa751cde
Size in memory: 240
References: 0
Number of entries: 1
Members:
142.251.46.164

I have confirmed that an mwan3 config referencing the "test-dnsmasq-population" ipset can be set to selectively route over a preferred WAN interface and that traffic follows the mwan3 rule as expected.

If there is something you believe to be broken in this config, please let me know, I'll be happy to test it.

2 Likes

Nothing I can see except you have to modify the dnsmasq.conf file.

  1. What happens if you had a package that sets up the dnsmasq uci config in the pre-23.05 way?

  2. What happens if you use fw4 to create one if its "ipsets" as well?

  3. Instead if recompiling dnsmasq, have you tried the ipset-dns package as tested by @vgaetera If that works with mwan3 then we have an out-of-the-box workaround.

Yes, I thought I had made that clear earlier, glad we are on the same page now. To reiterate an earlier point, while it may not be ideal, this sort of modification to /etc/dnsmasq.conf is a documented feature of OpenWRT and mwan3's docs discuss how to use it.

Do you have a package in mind that I could test? No packages I use do this. mwan3, in particular, has never offered a mechanism to create uci ipsets, its always been a manual process.

There is some confusion here because there are two meanings of "IP set" even in the uci sense, and I think we may have been referring to them differently all along.

  1. The option in "Network" -> "DHCP and DNS" -> "IP Sets" corresponding to 'config ipset' in /etc/config/dhcp

  2. The option in "Network" -> "Firewall" -> "IP Sets" corresponding to 'config ipset' in /etc/config/firewall.

Meaning (1) works with my patched dnsmasq-full, as expected in earlier comments in the thread. Both an ipset and nftset configuration line appear in /var/etc/dnsmasq.conf.cfg. Consistent with prior behavior, and the dnsmasq man page, these sets do not immediately appear 'nft list sets' or 'ipsets list", they must be created separately. However, once either (or both) sets are created the patched dnsmasq-full populates both types of set with resolved IPs following its configuration file.

Meaning (2) is one way of creating ipsets for dnsmasq to populate. Creating a "firewall" IP set results in an nft set that appears in "nft list sets". The Luci created set does not appear in "ipset list". The latter means its not visible to mwan3 as well. To make an iptables-style ipset the ipset must be created externally to both dnsmasq and the firewall. Historically I've done that from rc.local, based on a recommendation I believe came from this forum when it became clear mwan3 would not be migrating to nftables immediately.

I think @vgaetera demonstrated it works. It shares the same disadvantage that the iptables ipset must be created in rc.local (or similarly) for dnsmasq to populate it. Beyond that its just an extra layer of indirection, but I agree it's a workaround if nothing else changes (no change in dnsmasq-full build options, no immediate port of mwan3 to nftables).

However, to loop all the way around to an earlier point. As near as I can tell this is a one line change to the dnsmasq-full build configuration. It appears to cause no harm (nothing breaks), and the limitations of interop with the "firewall" version of IP sets in uci/luci are not new with 23.05. Presumably the mwan3 documentation could be updated to explain the use of rc.local to create the ipsets for dnsmasq and mwan3 to share.

Am I missing anything that would prevent this one-line change from being a viable workaround to be discussed in the github issue (I'm not clear who would make this decision)?

2 Likes

It was new with the introduction of fw4, because fw4 works entirely with nftables.

Ipset support is known throughout the Linux world, It uses the ipset utility to maintain an external database of ip addresses, as an extension to iptables, so of course will not work with Fw4.

The use of the ipset-dns package could provide an "out of the box" solution to the mwan3 problem. The vast majority of people do not want to or even know how to recompile dnsmasq. They want to install a fully working mwan3.

This would be a "documentation only" change and would be a short term workaround because mwan3 will either be migrated in due course or become obsolete.

I will defer to @jamesmacwhite, who added the warning to the mwan3 documentation linking to this discussion as to whether the docs should make a specific recommendation. I think his list of options in this post remains relevant, as ipset-dns is just a variant on option 2 in that list.

For now I plan to revert to 22.03 and wait to see a decision on the github issue .

The warning was added quite hastily given the recent release of 23.05 and making sure it was identified before anyone who went ahead and upgraded and then found ipset functionalty breaking. No doubt others have found this already prior, but at least to get some disclaimer present going forward so it is made clear.

As to the workarounds, happy to add temporary options/solutions into the docs although the best advice for users is likely to stay on 22.03 for simplicity given all the variables. While OpenWrt generally is aimed at the more technicial userbase, not everyone wants to spend hours configuring and editing configs outside of LuCI, which any workaround currently will require a degree of. Most of us know the fw4 and nftables story, but many users won't, so there is a risk of complicating factors.

At this point, it seems like spending time and resource for mwan3 to be updated to natively support nftables, focussing on the core problem. Happy to update the docs with any workarounds and temporary stop gaps though.

1 Like

I apologize for resurrecting an old thread.

Warning: This is a total kludge, It is a "this needs to work" solution meant to persist between updates and not require that I build and package my own dnsmasq-full package. This is not what what good looks like.

I have several mwan3 rules that are just ineffective without ipset (or some kind of set) support. In order to make things more-or-less work with 23.05.* I cobbled together this solution:

Packages: mwan3 dnsmasq-full

In System -> Startup -> Local Startup:

# Create ipset rules
# IPSet for HBOMax
ipset create hbomax hash:ip
[...]

In Network -> DHCP and DNS -> IP Sets (this seems misnamed since it creates nft sets):
Name, then FQDN matches, Netfilter and IP Family are left at default. I.e.:

hbomax       hbomaxcdn.com        fw4        IPv4+6
             api.hbo.com
             h264.io

I then use the following script to sync the nft ruleset with ipset every 10 seconds. In cron I have it set to run once per minute, though each launch will check to see if the script is already running and, if it is, will self-terminate.

This script dumps the entries in the nft rulesets to a simple text file in /tmp/ It compares the dump from 10 seconds ago against the current dump, identifies whatever is new, and imports those into the kernel's current ipsets (these were created at startup earlier). If it find an ipset specified (somehow dnsmasq is populating one that we forgot to create) then it will create the missing set.

This script is intentionally using /tmp in order to avoid frequent writes to persistent storage. There will be no persistence of the ipsets (or nft sets) between reboots.

#!/bin/bash

# Check if the script is already running
pid=$$
script=$(basename $0)
guard="/tmp/$script-$(id -nu).pid"
if test -f $guard ; then
    echo >&2 "ERROR: Script already runs... own PID=$pid"
    ps | grep $script | grep -v grep >&2
    exit 1
fi
trap "rm -f $guard" EXIT
echo $pid >$guard

# Get fresh data every 10 seconds
# Populate our old, new, and import sets
# We want this while loop to persist forever
while true; do
    # Remove old files
    rm -rf /tmp/set2 /tmp/set_import

    # If set1 exists, rename it to set2
    if [ -e "/tmp/set1" ]; then
        mv /tmp/set1 /tmp/set2
    else
        # If set1 doesn't exist, create a 0 length file called set2
        touch /tmp/set2
    fi

    # Find the contents of the nft set tables and export them to a text file
    nft list ruleset |\
        awk '/set .* \{/ {ipset=$2; getline; while ($1 != "}") {gsub(/[{};]/, ""); print $1, ipset; getline}}' |\
        sed 's/,//' |\
        sed -n '/^[0-9]/p' > /tmp/set1

    # Compare new data in set1 against older data in set2 and save the delta to set_import
    # We are looking for new entries by looking for the lines that start with "+"
    # We are stripping anything that doesn't start with "+" and are then stripping the "+" itself
    diff -u /tmp/set2 /tmp/set1 | sed -n '/^+[^+]/s/^+//p' > /tmp/set_import

    # Process the IP sets from set_import
    # Take the entries we've identified as new and import them to ipset
    while IFS= read -r line; do
        if [ -z "$line" ]; then
            continue
        fi

        ip_address=$(echo "$line" | awk '{print $1}')
        ipset_name=$(echo "$line" | awk '{print $2}')

        if [ -z "$ip_address" ] || [ -z "$ipset_name" ]; then
            echo "Malformed line: $line. Skipping."
            continue
        fi

        # Check if the IPset exists
        ipset_exists=$(ipset list -n "$ipset_name")

        # If IPset does not exist, create it
        if [ -z "$ipset_exists" ]; then
            ipset create "$ipset_name" hash:ip
            echo "Created IPset: $ipset_name"
        fi

        # Add IP address to the IPset if not already added
        ipset test "$ipset_name" "$ip_address" >/dev/null 2>&1
        if [ $? -ne 0 ]; then
            ipset add -q "$ipset_name" "$ip_address"
            logger "Added IP address to $ipset_name: $ip_address"
        fi
    done < /tmp/set_import

    # Sleep for 10 seconds before the next iteration
    sleep 10
done

Improvements for the future could be a persistent list of the nft rulesets that I care about and have the script auto-create the ipsets as needed (the script is already capable of doing this, so strictly speaking creating the ipsets in Startup -> Local Startup is not needed). Persisting the ipset (and nft ruleset) could be contemplated by writing the files to persistent storage on some regular interval (60 min?) and importing that set at startup.

I've picked up on the kludge script and optimized it a bit using nftables monitor functionality (reducing update latency by a lot). It now also creates ipsets on startup and adds new ipsets dynamically:

#!/bin/bash 
#check if the script is already running
PID=$$
SCRIPT=$(basename $0)
GUARD="/tmp/$script-$(id -nu).pid"
if [ -e $GUARD ]; then
  echo >&2 "ERROR: Script already runs... own PID=$PID"
  ps | grep $SCRIPT | grep -v grep >&2
  exit 1
fi
trap "rm -rf $GUARD" EXIT
echo $pid >$GUARD

# Check if ipsets exist for all currently existing ipsets or create otherwise
nft -tnT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set" | while read LINE; do
  NAME="$(echo $LINE | cut -d' ' -f2)"
  if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
    OPTS=" "
    if echo "$DEF" | grep -q "ipv6_addr"; then
      OPTS="$OPTS family inet6"
    fi 
    TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/')"
    if echo "$TIMEOUT" | grep -q "^[0-9]+$"; then
      OPTS="$OPTS timeout $TIMEOUT"
    fi
    ipset create "$NAME" hash:ip $OPTS
  fi
done
# (Optional)This might be needed here for mwan3 to work properly:
#mwan3 restart

# Monitor NFT rule changes
nft -nT monitor | while read LINE; do
  if echo "$LINE" | grep -q "^#"; then
    # Ignore comments
    true
  elif echo "$LINE" | grep -q "add set inet fw4"; then
    # Create ipset if it does not exist already
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
      OPTS=" "
      if echo "$DEF" | grep -q "ipv6_addr"; then
        OPTS="$OPTS family inet6"
      fi 
      TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/')"
      if echo "$TIMEOUT" | grep -q "^[0-9]+$"; then
        OPTS="$OPTS timeout $TIMEOUT"
      fi
      ipset create "$NAME" hash:ip $OPTS
    fi
  elif echo "$LINE" | grep -q "add element inet fw4"; then
    # Add element to the ipset (or update it)
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    IP="$(echo "$LINE" | cut -d' ' -f 7)"
    EXPIRES="$(echo "$LINE" | cut -d' ' -f 9)"

    # Check if set exists or create otherwise
    if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
      DEF="$(nft -tnT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "set $NAME")"
      OPTS=" "
      if echo "$DEF" | grep -q "ipv6_addr"; then
        OPTS="$OPTS family inet6"
      fi 
    
      TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/')"
      if echo "$TIMEOUT" | grep -q "^[0-9]+$"; then
        OPTS="$OPTS timeout $TIMEOUT"
      fi
      ipset create "$NAME" hash:ip $OPTS
    fi
  
    # Add element to ipset
    if ipset -q test "$NAME" "$IP"; then
      # Refresh the entry by deleting it first if already existing
      ipset -q del "$NAME" "$IP"
      ipset -q add "$NAME" "$IP"
    else   
      ipset -q add "$NAME" "$IP" 2>&1
      logger "Added $IP to ipset $NAME"
      # (Optional, requires pkg conntrack) Since we are using mwan3: clear all conntrack entries to this dest-ip (comment out if unneeded)
      ( conntrack -D -d "$IP" || true ) | grep -v "0 flow entries" | logger
    fi
  #else
  #  logger "Found unknown entry in nftables monitor: $LINE"
  fi
done

There are a few commented out things in there that might be helpful (conntrack clearing is enabled unless commented out but only works if conntrack is installed). So far its working for me with almost no CPU overhead. Tempfiles are also unnecessary this way. It is important that the ipsets are created properly (with familiy inet6 for IPv6 ipsets and also have the correct timeout set upon creation analog to the nft sets).

Nice job @Kishi ! Your script is a real improvement on my hack. Thanks for the update!

I think there's a bug in the "is running?" section - you set SCRIPT as a variable, then reference $script when setting the GUARD variable path to check. Functionally I think it'll still work, but the guard/lock file will not have a filename that makes sense if you happen to look in /tmp. I think you'd end up with /tmp/-root.pid rather than something like /tmp/nft_ipset-root.pid .

I've added some logger statements where it creates sets, adds addresses, or refreshes addresses.

I notice that you reference $DEF multiple times but don't declare it. Are you expecting this to be set by some other process? It looks like you want the admin to set this as a variable at the top of the script with possible values of ipv6_addr and timeout ### (where ### is a number in seconds). I presume it would be valid to set it like:

DEF=ipv6_addr timeout 300

There were some issues in the last code I've posted (mostly due to massive refactoring before posting it here). Today I've found some time to clean everything up and also remove the requirement for the local startup pre-staging. It might take until the next full minute for mwan3 to work properly upon boot though due to missing ipsets but if the script can be turned into a Init-Service that is started before firewall/mwan3 is started (prio 18 i guess) this will also be fixed (next tasks on my list).

Here's my current version:

#!/bin/bash 
#check if the script is already running
PID=$$
SCRIPT="$(basename $0)"
GUARD="/tmp/$SCRIPT-$(id -nu).pid"
if [ -e "$GUARD" ]; then
  echo >&2 "ERROR: Script already runs... own PID=$PID"
  ps | grep "$SCRIPT" | grep -v grep >&2
  exit 1
fi
trap "rm -rf $GUARD" EXIT
echo "$PID" > "$GUARD"

create_or_update_ipset() {
  # Determine ipset parameters
  local DEF="$1"
  local NAME="$(echo "$DEF" | cut -d' ' -f2)"
  local OPTS=" "
  local FAMILY="inet"
  if echo "$DEF" | grep -q "ipv6_addr"; then
    FAMILY="inet6"
    OPTS="$OPTS family $FAMILY"
  fi 
  local TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/; t; s/.*/0/')"
  if [ -n "$TIMEOUT" -a "$TIMEOUT" -gt 0 ]; then
    OPTS="$OPTS timeout $TIMEOUT"
  fi

  # Create or update ipset from nftables set
  if [ "$(ipset list -n "$NAME")" = "$NAME" ]; then
    CUR="$(ipset list -t "$NAME")"
    if ! ( echo "$CUR" | grep -q "family $FAMILY"); then
      ( ipset destroy "$NAME" | logger ) || logger "WARNING: Could not destroy ipset with family != $FAMILY"    
    elif ! ( echo "$CUR" | grep -q "timeout $TIMEOUT"); then
      # Swap current iteration of the ipset with a new iteration due to timeout mismatch
      ipset create "_$NAME" hash:ip $OPTS
      ipset swap "_$NAME" "$NAME"
      ipset destroy "_$NAME"
      logger "Replaced ipset $NAME with new iteration with timeout $TIMEOUT"
    fi
  fi
  if [ "$(ipset list -n "$NAME")" != "$NAME" ]; then
    # Create a new ipset with options matching the nftables set
    ipset create "$NAME" hash:ip $OPTS
    logger "Created new ipset $NAME with timeout $TIMEOUT"
    # Restart mwan3 if this ipset is used by it and it is already running (but active rules will not have properly setup yet due to missing ipset)
    if [ $? = 0 ] && cat '/etc/config/mwan3' 2>/dev/null | grep -q "ipset '$NAME'" && ( service | grep mwan3 | grep running ); then
      mwan3 restart
    fi
  fi

  # Add already existing entries to the set
  echo "$DEF" | sed -re 's/.*elements = \{ ([^\}]+) \}.*/\1/g' | tr ',' '\n' | sed -re 's/^[ ]+//g;s/expires/timeout/g;s/s$//g' | while read LINE; do
    ipset -q add "$NAME" $LINE >/dev/null 2>&1 && logger "Added $LINE to $NAME upon ipset creation/update"
  done
}

# Check if ipsets exist for all currently existing nftsets or create otherwise
nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set" | while read DEF; do
  create_or_update_ipset "$DEF"
done

# Monitor nftables rule changes
nft -nT monitor | while read LINE; do
  if echo "$LINE" | grep -q "add element inet fw4"; then
    # Check if ipset exists or create otherwise
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
      DEF="$(nft -tnT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
      create_or_update_ipset "$DEF"
    fi
  
    IP="$(echo "$LINE" | cut -d' ' -f 7)"
    EXPIRES="$(echo "$LINE" | cut -d' ' -f 9 | grep -Eo '[0-9]*')"
    ADDOPTS=""
    if [ -n "$EXPIRES" -a "$EXPIRES" -gt 0 ]; then
      ADDOPTS="timeout $EXPIRES"
    fi
    # Add element to ipset
    if ipset -q test "$NAME" "$IP"; then
      # Refresh the entry by deleting it first if already existing
      ipset -q del "$NAME" "$IP"
      ipset -q add "$NAME" "$IP" $ADDOPTS
    else
      ipset -q add "$NAME" "$IP" 2>&1
      logger "Added $IP to ipset $NAME" $ADDOPTS
    fi
  elif echo "$LINE" | grep -q "add set inet fw4"; then
    # Create or update ipset
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    DEF="$(nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
    create_or_update_ipset "$DEF"
  elif echo "$LINE" | grep -q "delete set inet fw4"; then
    # Clear and try to delete removed ipset
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    ipset clear "$NAME"
    ipset destroy "$NAME" 2>&1 | logger
  fi
done
# vim: ts=2 sw=2 et

This will mirror any nftables sets to ipset ones by just running it with minutely started cron job. Most important set changes are also covered:

  • Adding entries (with proper timeout handling, so those are mirrored)
  • Adding new sets
  • Deleting sets
  • Modifying sets: Changing timeout and IPv4<->IPv6 (but only if ipset is unreferenced by iptables/mwan3)

At this point it is enough for me to be able to use ipsets in mwan3 properly on 23.05 even though they are propagated to nftable sets by dnsmasq-full.

1 Like

Excellent update! Thank you!

You might consider changing the TIMEOUT sed magic to:

  local TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/; t; s/.*/0/')"

This will capture the situation where no timeout is specified in the set and, in this case, set the value to "0". Without that TIMEOUT is carrying the entire set value that was passed in DEF.

Really dumb question - where are you setting the timeout value for the nft sets? Is this an option in the /etc/foncif/dhcp file that I'm overlooking?

1 Like

Thanks for noting on that edge case. I'll look into it and I might even have a simpler solution for it after now having tested the script in it's current state for a bit. Should that not work I'll gladly pick up on your sed magic.

The timeout setting for the NFT set is available when defining the set in the firewall (in /etc/config/firewall) or under Firewall > IP Sets in LUCI. It is an option that can be defined separately for each set.

I'm making some progress on turning this into a portable init-Script as well. I'll post that once it is in a sufficiently working and tested state, removing the current cron-based launching with a more standard way of launching it.

EDIT: I've included your sed magic in the script in my previous post so that it can be simply copy&pasted. Thanks again.

Aha! I've been trawling through IP Sets in /etc/config/dhcp and Network -> DHCP & DNS and not seeing it anywhere. Didn't even occur to me to look in Network->Firewall (though, obviously, I once did since the IPSets are all there). Thanks!

I've noticed another "Expected an integer" error -

EXPIRES="$(echo "$LINE" | cut -d' ' -f 9 | grep -Eo '[0-9]*')"
...
if [ -n "$EXPIRES" -a "$EXPIRES" -gt 0 ]; then

It seems as though the -n "$EXPIRES" isn't suppressing the -gt evaluation and is causing if to be grumpy. Changing the evaluation to something like this will make sure the -gt happens only if $EXPIRES is holding actual integers:

if [[ "$EXPIRES" =~ ^[0-9]+$ && "$EXPIRES" -gt 0 ]]; then
1 Like

Thanks for the expires fix. I've integrated it into the latest iteration. Which is now also an init Script and does not need to be run using cron anymore (although it's basically simply wrapping the script into a init-service):

#!/bin/bash /etc/rc.common
# Start before firewall and mwan3 which are at Prio 19
START=18
APP=nft2ipset
USE_PROCD=1
SCRIPTPATH="/tmp/nft2ipset"

write_script() {
cat > "$1" <<'EOT'
#!/bin/bash
#check if the script is already running
PID=$$
SCRIPT="$(basename $0)"
TMPDIR="/tmp"
MONITORPIDFILE="$TMPDIR/$SCRIPT-$$.nftmonitorpid"
MONITORFIFO="$TMPDIR/$SCRIPT-$$.nftmonitorfifo"
mkfifo "$MONITORFIFO"

cleanup () {
  # Cleanup nft monitor subprocess
  if [ -f "$MONITORPIDFILE" ]; then
    MONITORPID="$(cat "$MONITORPIDFILE")"
    if [ "$MONITORPID" -gt 1 ]; then
      kill "$MONITORPID"
    fi
  fi
  # Remove pid file and fifo
  rm "$MONITORFIFO" "$MONITORPIDFILE"
}
trap cleanup TERM INT EXIT

create_or_update_ipset() {
  # Determine ipset parameters
  local DEF="$1"
  local NAME="$(echo "$DEF" | cut -d' ' -f2)"
  local OPTS=""
  local FAMILY="inet"
  if echo "$DEF" | grep -q "ipv6_addr"; then
    FAMILY="inet6"
    OPTS="$OPTS family $FAMILY"
  fi 
  local TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/; t; s/.*/0/')"
  if [ -n "$TIMEOUT" -a "$TIMEOUT" -gt 0 ]; then
    OPTS="$OPTS timeout $TIMEOUT"
  fi

  # Create or update ipset from nftables set
  if [ "$(ipset list -n "$NAME")" = "$NAME" ]; then
    CUR="$(ipset list -t "$NAME")"
    if ! ( echo "$CUR" | grep -q "family $FAMILY"); then
      ( ipset destroy "$NAME" 2>&1 | logger -t "$SCRIPT" ) || logger -t "$SCRIPT" "WARNING: Could not destroy ipset with family != $FAMILY"    
    elif ! ( echo "$CUR" | grep -q "timeout $TIMEOUT"); then
      # Swap current iteration of the ipset with a new iteration due to timeout mismatch
      ipset create "_$NAME" hash:ip $OPTS
      ipset swap "_$NAME" "$NAME"
      ipset destroy "_$NAME"
      logger -t "$SCRIPT" "Replaced ipset $NAME with new iteration with timeout $TIMEOUT"
    fi
  fi
  if [ "$(ipset list -n "$NAME")" != "$NAME" ]; then
    # Create a new ipset with options matching the nftables set
    ipset create "$NAME" hash:ip $OPTS
    # Restart mwan3 if this ipset is used by it, it is already running but the set name is not found in active rule output
    if [ $? = 0 ] && grep -q "option ipset '$NAME'" /etc/config/mwan3 2>/dev/null && ( service | grep mwan3 | grep running ) && ( ! (mwan3 rules | grep -q "match-set $NAME" ) ); then
      mwan3 restart
    fi
    logger -t "$SCRIPT" "Created new ipset $NAME with timeout $TIMEOUT"
  fi

  # Add already existing entries to the set
  echo "$DEF" | sed -re 's/.*elements = \{ ([^\}]+) \}.*/\1/g' | tr ',' '\n' | sed -re 's/^[ ]+//g;s/expires/timeout/g;s/s$//g' | while read LINE; do
    ipset -q add "$NAME" $LINE >/dev/null 2>&1 && logger -t "$SCRIPT" "Added $LINE to $NAME upon ipset creation/update"
  done
}

# Check if ipsets exist for all currently existing nftsets or create otherwise
nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set" | while read DEF; do
  create_or_update_ipset "$DEF"
done

# Monitor nftables rule changes
nft -nT monitor > "$MONITORFIFO" 2>&1 &
echo $! > "$MONITORPIDFILE"
while read LINE; do
  if echo "$LINE" | grep -q "add element inet fw4"; then
    # Check if ipset exists or create otherwise
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
      DEF="$(nft -tnT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
      create_or_update_ipset "$DEF"
    fi
    # Add element to ipset
    IP="$(echo "$LINE" | cut -d' ' -f 7)"
    EXPIRES="$(echo "$LINE" | cut -d' ' -f 9 | grep -Eo '[0-9]*')"
    ADDOPTS=""
    if [[ "$EXPIRES" =~ ^[0-9]+$ && "$EXPIRES" -gt 0 ]]; then
      ADDOPTS="timeout $EXPIRES"
    fi
    if ipset -q test "$NAME" "$IP"; then
      # Refresh the entry by deleting it first if already existing
      ipset -q del "$NAME" "$IP"
      ipset -q add "$NAME" "$IP" $ADDOPTS
    else
      ipset -q add "$NAME" "$IP" $ADDOPTS
      logger -t "$SCRIPT" "Added $IP to ipset $NAME"
    fi
  elif echo "$LINE" | grep -q "add set inet fw4"; then
    # Create or update ipset 
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    DEF="$(nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
    create_or_update_ipset "$DEF"
  elif echo "$LINE" | grep -q "delete set inet fw4"; then
    # Clear and try to delete removed ipset (This will fail if it is in use by any iptables rule)
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    ipset clear "$NAME"
    ipset destroy "$NAME" 2>&1 | logger -t "$SCRIPT"
  fi
done < "$MONITORFIFO"
EOT
}

start_service() {
  write_script "$SCRIPTPATH"
  chmod +x "$SCRIPTPATH"
  procd_open_instance
  procd_set_param command "$SCRIPTPATH"
  procd_set_param respawn
  procd_close_instance
}
service_stopped() {
  rm "$SCRIPTPATH"
}
# vim: ts=2 sw=2 et

For this to work you simply have to put the contents above into /etc/init.d/nft2ipset and enable + start the new service with:

chmod +x /etc/init.d/nft2ipset
service nft2ipset enable
service nft2ipset start

I've tested functionality and reboot safety so this should be as simple and portable as it gets to make mwan3 work on 23.05+ without it being adapted to nftables.

EDIT: The old cron scheduling and script can be safely removed after enabling/starting the new service.
P.S.: The new FIFO in the script is to work around some interesting way that bash handles subprocess pipes (nft monitor in our case) that would not properly cleanup on killing the script otherwise.

EDIT2: Added logging tag for messages of the script and missing note on making this executable

2 Likes

Awesome job! Thank you!

I've done some minor updates to the logger statements (making each message start with "$SCRIPT: to make the entries easier to find, adding a message about "filling new IPset with existing nft entries," and clarifying the recreated-ipset-with-$OPT message).

Oh! One note to future users that might go this path - once you drop this into /etc/init.d/nft2ipsets don't forget to make the script executable with:

chmod +x /etc/init.d/nft2ipset

and then follow the instructions above to enable and start the script.

I'm now happily using this. Thank you!

I wonder if @feckert might be interested in integrating this into mwan3 at least until he can get the tool to natively understand nft sets.

1 Like

I've updated my last post and added logger tags (basically what you've done as well with logger statements on your end @jmccabe06 or probably the same actually) and noted on making the init script executable so necessary information is compiled in one spot.

Thanks again for testing and suggesting things that can be improved!

An additional note for everyone wanting to try the script: It has only been tested and was coded around bash specifically (and also uses features that are present in bash like the extended test syntax for wildcard testing). You'll have to install bash (opkg install bash) before being able to use it. The service wrapper has it's shebang set to bash as well for this reason, so you'd only be able to use the service if bash is present. If you're using the script without bash (e.g. with the default openwrt /bin/sh) I have not tested this case and it might break at specific points erroring out.

Here's a new version that works without bash (didn't take much to get it there). Tested with default /bin/sh from OpenWRT 23.05:

#!/bin/sh /etc/rc.common
# Start before firewall and mwan3 which are at Prio 19
START=18
APP=nft2ipset
USE_PROCD=1
SCRIPTPATH="/tmp/nft2ipset"

write_script() {
cat > "$1" <<'EOT'
#!/bin/sh
#check if the script is already running
PID=$$
SCRIPT="$(basename $0)"
TMPDIR="/tmp"
MONITORPIDFILE="$TMPDIR/$SCRIPT-$$.nftmonitorpid"
MONITORFIFO="$TMPDIR/$SCRIPT-$$.nftmonitorfifo"
mkfifo "$MONITORFIFO"

cleanup () {
  # Cleanup nft monitor subprocess
  if [ -f "$MONITORPIDFILE" ]; then
    MONITORPID="$(cat "$MONITORPIDFILE")"
    if [ "$MONITORPID" -gt 1 ]; then
      kill "$MONITORPID"
    fi
  fi
  # Remove pid file and fifo
  rm "$MONITORFIFO" "$MONITORPIDFILE"
}
trap cleanup TERM INT EXIT

create_or_update_ipset() {
  # Determine ipset parameters
  local DEF="$1"
  local NAME="$(echo "$DEF" | cut -d' ' -f2)"
  local OPTS=""
  local FAMILY="inet"
  if echo "$DEF" | grep -q "ipv6_addr"; then
    FAMILY="inet6"
    OPTS="$OPTS family $FAMILY"
  fi 
  local TIMEOUT="$(echo "$DEF" | sed -r 's/.*timeout ([0-9]*)s.*/\1/; t; s/.*/0/')"
  if [ -n "$TIMEOUT" -a "$TIMEOUT" -gt 0 ]; then
    OPTS="$OPTS timeout $TIMEOUT"
  fi

  # Create or update ipset from nftables set
  if [ "$(ipset list -n "$NAME")" = "$NAME" ]; then
    CUR="$(ipset list -t "$NAME")"
    if ! ( echo "$CUR" | grep -q "family $FAMILY"); then
      ( ipset destroy "$NAME" 2>&1 | logger -t "$SCRIPT" ) || logger -t "$SCRIPT" "WARNING: Could not destroy ipset with family != $FAMILY"    
    elif ! ( echo "$CUR" | grep -q "timeout $TIMEOUT"); then
      # Swap current iteration of the ipset with a new iteration due to timeout mismatch
      ipset create "_$NAME" hash:ip $OPTS
      ipset swap "_$NAME" "$NAME"
      ipset destroy "_$NAME"
      logger -t "$SCRIPT" "Replaced ipset $NAME with new iteration with timeout $TIMEOUT"
    fi
  fi
  if [ "$(ipset list -n "$NAME")" != "$NAME" ]; then
    # Create a new ipset with options matching the nftables set
    ipset create "$NAME" hash:ip $OPTS
    # Restart mwan3 if this ipset is used by it, it is already running but the set name is not found in active rule output
    if [ $? = 0 ] && grep -q "option ipset '$NAME'" /etc/config/mwan3 2>/dev/null && ( service | grep mwan3 | grep running ) && ( ! (mwan3 rules | grep -q "match-set $NAME" ) ); then
      mwan3 restart
    fi
    logger -t "$SCRIPT" "Created new ipset $NAME with timeout $TIMEOUT"
  fi

  # Add already existing entries to the set
  echo "$DEF" | sed -re 's/.*elements = \{ ([^\}]+) \}.*/\1/g; t; s/.*//g' | tr ',' '\n' | sed -re 's/^[ ]+//g;s/expires/timeout/g;s/s$//g' | while read LINE; do
    if [ -n "$LINE" ]; then
      ipset -q add "$NAME" $LINE && logger -t "$SCRIPT" "Added $LINE to $NAME upon ipset creation/update" || true
    fi
  done
}

# Check if ipsets exist for all currently existing nftsets or create otherwise
nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set" | while read DEF; do
  create_or_update_ipset "$DEF"
done

# Monitor nftables rule changes
nft -nT monitor > "$MONITORFIFO" 2>&1 &
echo $! > "$MONITORPIDFILE"
while read LINE; do
  if echo "$LINE" | grep -q "add element inet fw4"; then
    # Check if ipset exists or create otherwise
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    if [ "$(ipset list -n $NAME)" != "$NAME" ]; then
      DEF="$(nft -tnT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
      create_or_update_ipset "$DEF"
    fi
    # Add element to ipset
    IP="$(echo "$LINE" | cut -d' ' -f 7)"
    EXPIRES="$(echo "$LINE" | sed -re 's/.*expires ([0-9]+)s.*/\1/; t; s/.*/0/')"
    ADDOPTS=""
    if [ $EXPIRES -gt 0 ]; then
      ADDOPTS="timeout $EXPIRES"
    fi
    if ipset -q test "$NAME" "$IP"; then
      # Refresh the entry by deleting it first if already existing
      ipset -q del "$NAME" "$IP"
      ipset -q add "$NAME" "$IP" $ADDOPTS
    else
      ipset -q add "$NAME" "$IP" $ADDOPTS
      logger -t "$SCRIPT" "Added $IP to ipset $NAME $ADDOPTS"
    fi
  elif echo "$LINE" | grep -q "add set inet fw4"; then
    # Create or update ipset 
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    DEF="$(nft -nT list sets | tr '\n' ' ' | awk '{$1=$1;print}' | sed -r 's/(set|table)/\n\1/g' | grep "^set $NAME")"
    create_or_update_ipset "$DEF"
  elif echo "$LINE" | grep -q "delete set inet fw4"; then
    # Clear and try to delete removed ipset (This will fail if it is in use by any iptables rule)
    NAME="$(echo "$LINE" | cut -d' ' -f 5)"
    ipset clear "$NAME"
    ipset destroy "$NAME" 2>&1 | logger -t "$SCRIPT"
  fi
done < "$MONITORFIFO"
EOT
}

start_service() {
  write_script "$SCRIPTPATH"
  chmod +x "$SCRIPTPATH"
  procd_open_instance
  procd_set_param command "$SCRIPTPATH"
  procd_set_param respawn
  procd_close_instance
}
service_stopped() {
  rm "$SCRIPTPATH"
}
# vim: ts=2 sw=2 et

Copy the script contents above to /etc/init.d/nft2ipset and run the following to enable the service:

chmod +x /etc/init.d/nft2ipset
service nft2ipset enable
service nft2ipset start

Feel free to leave feedback if you find any issues whilst testing/using this script. I've also posted a link to this script on the related Github issue (https://github.com/openwrt/packages/issues/22474) for mwan3 and it should be included on the mwan3 wiki page soonish.