Release Asuswrt-Merlin 3006.102.4 is now available

[kuchkovsky]

Have you tried on different browser?

Yes. Safari, Chrome, Firefox - no difference. My router model is the RT-BE88U, it should be powerful enough

 

[bennor]

@RMerlin while not specific only to this release, I noticed that rendering the second half of the System Info page (starting from Memory, Swap, etc.) takes around 20 seconds when there are many connections (22466 / 300000 with 4342 active, specifically).

Would it be possible to optimize this? Perhaps by separating the logic that counts connections from the rest of the rendering, so the main page loads instantly and the connection count updates asynchronously?

Is this happening prior to installing any addon scripts including those you just mentioned in your other post?

 

[kuchkovsky]

Is this happening prior to installing any addon scripts including those you just mentioned in your other post?

Yes. I tried disabling them, no difference. These scripts are lightweight, event-based, and don't affect that particular page. The CPU is not loaded.

 

[RMerlin]

@RMerlin while not specific only to this release, I noticed that rendering the second half of the System Info page (starting from Memory, Swap, etc.) takes around 20 seconds when there are many connections (22466 / 300000 with 4342 active, specifically).

Would it be possible to optimize this? Perhaps by separating the logic that counts connections from the rest of the rendering, so the main page loads instantly and the connection count updates asynchronously?

There's nothing I can do to really optimize that any further. The router needs to read 22466 lines of text, search each line to see if it's "ESTABLISHED" , or "udp" and "ASSURED". I could possibly drop the "udp" search, to limit it to either ESTABLISHED or ASSURED regardless of the protocol, but it would probably only improve performance by roughly 33%.

                } else if(strcmp(type,"conn.active") == 0) {
                        char buf[256];
                        FILE* fp;
                        unsigned int established = 0;

                        fp = fopen("/proc/net/nf_conntrack", "r");
                        if (fp) {
                                while (fgets(buf, sizeof(buf), fp) != NULL) {
                                if (strstr(buf,"ESTABLISHED") || ((strstr(buf,"udp")) && (strstr(buf,"ASSURED"))))
                                        established++;
                                }
                                fclose(fp);
                        }
                        sprintf(result,"%u",established);

The only altnerative is to completely remove that information from the page.

 

[dave14305]

Perhaps by separating the logic that counts connections from the rest of the rendering, so the main page loads instantly and the connection count updates asynchronously

The connection info is already loaded via ajax. Does it take 20 seconds to load ajax_sysinfo.asp in the browser?

 

[RMerlin]

The connection info is already loaded via ajax. Does it take 20 seconds to load ajax_sysinfo.asp in the browser?

It's probably httpd that needs to read and do three string searches on every single of these 22000 lines.

 

[kuchkovsky]

Does it take 20 seconds to load ajax_sysinfo.asp in the browser?

Yes. That's why I suggested separating this logic into a separate ajax call, so it doesn't freeze the rest of the page.

 

[RMerlin]

Dropping the "udp" search itself probably won't really help. But since tcp and udp are quite early in the line, I wonder if the following logic could speed things up:

if (("tcp" and "ESTABLISHED") || ("udp" and "ASSURED"))
    established++;

That way, ESTABLISHED wouldn't get searched on udp entries.

EDIT: ideally, if the proto search could be limited to the first 20 chars of the line, that would probably provide a more notable benefit.

 

[kuchkovsky]

That way, ESTABLISHED wouldn't get searched on udp entries.

It should probably help. Currently, most of my connections are UDP (I run a public NTP server):

/tmp/home/root# cat /proc/net/nf_conntrack | grep udp | wc -l
11965
/tmp/home/root# cat /proc/net/nf_conntrack | grep tcp | wc -l
1902

 

[RMerlin]

Let me write a short program that will let you test the performance, give me a few minutes.

 

[RMerlin]

Try running these on your router:

parse-test

admin@stargate:/tmp/home/root# time /tmp/new
Result: 123
real    0m 0.00s
user    0m 0.00s
sys    0m 0.00s
admin@stargate:/tmp/home/root# time /tmp/original
Result: 146
real    0m 0.00s
user    0m 0.00s
sys    0m 0.00s

The two functions look like this:

merlin@ubuntu-dev:~/tempo$ cat new.c original.c
#include <string.h>
#include <stdio.h>

int main() {
    char buf[256];
    char proto[24] = {0};

        FILE* fp;
        unsigned int established = 0;

        fp = fopen("/proc/net/nf_conntrack", "r");
        if (fp) {
            while (fgets(buf, sizeof(buf), fp) != NULL) {
            strncpy(proto, buf, 23);
                    if ((strstr(proto, "tcp") && strstr(buf,"ESTABLISHED")) || (strstr(proto,"udp") && strstr(buf,"ASSURED")))
                            established++;
                }
                fclose(fp);
        }
    printf("Result: %d\n", established);
}



#include <string.h>
#include <stdio.h>

int main() {
    char buf[256];
        FILE* fp;
        unsigned int established = 0;

        fp = fopen("/proc/net/nf_conntrack", "r");
        if (fp) {
            while (fgets(buf, sizeof(buf), fp) != NULL) {
                    if (strstr(buf,"ESTABLISHED") || ((strstr(buf,"udp")) && (strstr(buf,"ASSURED"))))
                            established++;
                }
                fclose(fp);
        }
    printf("Result: %d\n", established);
}

(EDIT: fixed an out-of-bound string access)

 

[MDM]

The only altnerative is to completely remove that information from the page.

Noooooo!! Please dont, yes it rarely needed but can be very useful!
And not to mention who opens that page nonstop...

 

[kuchkovsky]

Try running these on your router:

/tmp/home/root# time /tmp/new
Result: 4724
real    0m 14.02s
user    0m 0.02s
sys    0m 13.51s

/tmp/home/root# time /tmp/original
Result: 5066
real    0m 38.91s
user    0m 0.04s
sys    0m 37.99s

That's a huge difference

But I have to say it's a bit inconsistent, even with similar counts. Sometimes, the results are very similar, but other times, I see a nice improvement over the original implementation.

 

[kuchkovsky]

UPD: Interesting, I tried to compile these apps myself using a Linux VM on my M3 Mac, and it seems that simply copying a file to a temporary location and reading from it is much faster than reading directly from /proc/net/nf_conntrack. Reading from a temporary file is almost instant with any implementation.

new1.c

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

int main() {
    char buf[256];
    char proto[24] = {0};
    unsigned int established = 0;

    // copy conntrack data to /tmp/nf_conntrack
    system("cp /proc/net/nf_conntrack /tmp/nf_conntrack");

    // read from the temp file instead
    FILE* fp = fopen("/tmp/nf_conntrack", "r");
    if (fp) {
        while (fgets(buf, sizeof(buf), fp) != NULL) {
            strncpy(proto, buf, 23);

            if ((strstr(proto, "tcp") && strstr(buf, "ESTABLISHED")) ||
                (strstr(proto, "udp") && strstr(buf, "ASSURED"))) {
                established++;
            }
        }
        fclose(fp);
    }

    printf("Result: %d\n", established);
    return 0;
}

/tmp/home/root# time /tmp/new1
Result: 3984
real    0m 3.09s
user    0m 0.00s
sys    0m 2.95s
/tmp/home/root# time /tmp/new
Result: 3992
real    0m 8.24s
user    0m 0.01s
sys    0m 7.93s
/tmp/home/root# time /tmp/original
Result: 3901
real    0m 7.30s
user    0m 0.02s
sys    0m 7.23s

 

[RMerlin]

UPD: Interesting, I tried to compile these apps myself using a Linux VM on my M3 Mac, and it seems that simply copying a file to a temporary location and reading from it is much faster than reading directly from /proc/net/nf_conntrack. Reading from a temporary file is almost instant with any implementation.

There might be a lot of overhead since it's a system device rather than an actual file. Might be worth trying to buffer the content into memory first (Asuswrt has a libshared function to do that), and parsing from that buffer rather than doing an fgets() read from the device.

 

[RMerlin]

Try this version, which relies on libshared's read_whole_file() - not sure if it would work significantly better than doing a copy of the file like you did. I recopied the read_whole_file() code from libshared. The compiled file was uploaded as new2 on my shared folder.

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

extern char *read_whole_file(const char *target){
        FILE *fp;
        char *buffer, *new_str;
        int i;
        unsigned int read_bytes = 0;
        unsigned int each_size = 1024;

        if((fp = fopen(target, "r")) == NULL)
                return NULL;

        buffer = (char *)malloc(sizeof(char)*each_size);
        if(buffer == NULL){
                fclose(fp);
                return NULL;
        }
        memset(buffer, 0, each_size);

        while ((i = fread(buffer+read_bytes, each_size * sizeof(char), 1, fp)) == 1){
                read_bytes += each_size;
                new_str = (char *)malloc(sizeof(char)*(each_size+read_bytes));
                if(new_str == NULL){
                        free(buffer);
                        fclose(fp);
                        return NULL;
                }
                memset(new_str, 0, sizeof(char)*(each_size+read_bytes));
                memcpy(new_str, buffer, read_bytes);

                free(buffer);
                buffer = new_str;
        }

        fclose(fp);
        return buffer;
}

int main() {
    char *buffer;
    char line[256];
    char proto[24] = {0};
    unsigned int established = 0;
    char *src, *dst;
    int charcount = 0;

    buffer = read_whole_file("/proc/net/nf_conntrack");
    if (buffer) {
        src = buffer;
        while (*src) {
            charcount = 0;
            dst = line;

            while (*src && *src != '\n' && charcount < 255) {
                *dst++ = *src++;
                charcount++;
            }
            *dst = '\0';

            strncpy(proto, line, 23);
            if ((strstr(proto, "tcp") && strstr(line,"ESTABLISHED")) || (strstr(proto,"udp") && strstr(line,"ASSURED")))
                established++;

            if (*src == '\n') src++;
        }

        free(buffer);
    }
    printf("Result: %d\n", established);
}

(nevermind the formatting, I'm doing quick edits/copy/pastes within nano, so there's a mixture of tabs and spaces)

 

[kuchkovsky]

Try this version, which relies on libshared's read_whole_file()

It's even slower than the original version.

I was trying out some changes, and this one seems to be even faster than the one with a simple cp:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/stat.h>

#define READ_CHUNK   32768          // 32 KiB
#define LINE_WIN     256            // max bytes of line we examine
#define PROTO_LEN    23


// modified version
static char *read_whole_file(const char *path, size_t *out_len)
{
    int fd = open(path, O_RDONLY);
    if (fd < 0) return NULL;

    struct stat st;
    size_t cap = (fstat(fd, &st) == 0 && st.st_size > 0)
                 ? (size_t)st.st_size
                 : READ_CHUNK;

    char *buf = malloc(cap + 1);
    if (!buf) { close(fd); return NULL; }

    size_t len = 0;
    while (1) {
        if (len + READ_CHUNK > cap) {
            cap *= 2;
            char *tmp = realloc(buf, cap + 1);
            if (!tmp) { free(buf); close(fd); return NULL; }
            buf = tmp;
        }
        ssize_t r = read(fd, buf + len, READ_CHUNK);
        if (r < 0) { free(buf); close(fd); return NULL; }
        if (r == 0) break;
        len += (size_t)r;
    }
    close(fd);
    buf[len] = '\0';
    if (out_len) *out_len = len;
    return buf;
}


int main(void)
{
    size_t len = 0;
    char *buffer = read_whole_file("/proc/net/nf_conntrack", &len);
    unsigned int established = 0;

    if (buffer) {
        char proto[PROTO_LEN + 1];
        char line[LINE_WIN];
        char *p   = buffer;
        char *end = buffer + len;

        while (p < end) {
            char *nl = memchr(p, '\n', end - p);
            if (!nl) nl = end;
            size_t l_len = (size_t)(nl - p);

            size_t copy_len = l_len > LINE_WIN - 1 ? LINE_WIN - 1 : l_len;
            memcpy(line, p, copy_len);
            line[copy_len] = '\0';

            memcpy(proto, line, PROTO_LEN);
            proto[PROTO_LEN] = '\0';

            if ((strstr(proto, "tcp") && strstr(line, "ESTABLISHED")) ||
                (strstr(proto, "udp") && strstr(line, "ASSURED")))
                established++;

            p = (nl == end) ? end : nl + 1;
        }
        free(buffer);
    }

    printf("Result: %u\n", established);
    return 0;
}

 

[RMerlin]

I don't want to make changes to read_whole_file() since it's a shared function also used elsewhere throughout the code.

At this point for the sake of keeping things simple (AFAIK, your scenario is an extreme uncommon one, so I don't really want to overengineer that one function for the 5-10 persons in the world that might really need it) I'd rather settle down for the optimized protocol parsing, combined with a file copy.

 

[kuchkovsky]

I'd rather settle down for the optimized protocol parsing, combined with a file copy.

Yes, that's a good idea. It's working well enough.

 

[RMerlin]

Yes, that's a good idea. It's working well enough.

Here's the final version within httpd:

        } else if(strcmp(type,"conn.active") == 0) {
            char buf[256], proto[20];
            FILE* fp;
            unsigned int established = 0;

            eval("cp", "/proc/net/nf_conntrack", "/tmp/conntrack.tmp");

            fp = fopen("/tmp/conntrack.tmp", "r");
            if (fp) {
                while (fgets(buf, sizeof(buf), fp) != NULL) {
                    strlcpy(proto, buf, sizeof(proto));
                    if ((strstr(proto, "tcp") && strstr(buf, "ESTABLISHED")) ||
                        (strstr(proto, "udp") && strstr(buf, "ASSURED")))
                        established++;
                }
                fclose(fp);
            }
            unlink("/tmp/conntrack.tmp");
            sprintf(result,"%u",established);