Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FS#1425 - iproute2: tc qdisc produces incorrect output #6408

Closed
openwrt-bot opened this issue Mar 10, 2018 · 4 comments
Closed

FS#1425 - iproute2: tc qdisc produces incorrect output #6408

openwrt-bot opened this issue Mar 10, 2018 · 4 comments
Labels
flyspray

Comments

@openwrt-bot
Copy link

None:

AR71xx - Archer c7 v2

tc qdisc output is producing bizarre output:

tc qdisc
qdisc noqueue 0: dev lo root refcnt 4486716
qdisc fq_codel 0: dev eth1 root refcnt 4486716 limit 4498840p flows 4536204 quantum 4539856 target 5.0ms interval 100.0ms memory_limit 4Mb ecn
qdisc noqueue 0: dev br-lan root refcnt 4486716
qdisc noqueue 0: dev eth1.2 root refcnt 4486716
qdisc noqueue 0: dev br-wifi_guest root refcnt 4486716
qdisc noqueue 0: dev eth1.15 root refcnt 4486716
qdisc noqueue 0: dev wlan1 root refcnt 4486716
qdisc noqueue 0: dev wlan0 root refcnt 4486716
qdisc noqueue 0: dev wlan1-1 root refcnt 4486716
qdisc noqueue 0: dev wlan0-1 root refcnt 4486716

Note refcnt, (and for fq_codel) limit, flows & quantum are all highly unlikely values. For more curiosity factor, if you ask for json output from tc you get the correct results:

tc -j qdisc
[{
"kind": "noqueue",
"handle": "0:",
"dev": "lo",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "fq_codel",
"handle": "0:",
"dev": "eth1",
"root": true,
"refcnt": 2,
"options": {
"limit": 10240,
"flows": 1024,
"quantum": 1514,
"target": 4999,
"interval": 99999,
"memory_limit": 4194304,
"ecn": true
}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "br-lan",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "eth1.2",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "br-wifi_guest",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "eth1.15",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "wlan1",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "wlan0",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "wlan1-1",
"root": true,
"refcnt": 2,
"options": {}
},{
"kind": "noqueue",
"handle": "0:",
"dev": "wlan0-1",
"root": true,
"refcnt": 2,
"options": {}
}
]

That suggests the netlink data transfer mechanism used by iproute2/kernel is okay. Instead it looks like iproute2 tc's print_uint(PRINT_ANY.... ) functions are broken in some way, possibly only on some architectures.

The print_* functions are pre-processor expanded functions mixed across lib/json_print.c & include/json_print.h found in iproute2.

I'm afraid I can't work out what's going on/wrong here.
@openwrt-bot
Copy link
Author

None:

Forgot to mention - tried compiling with gcc5 - no change.

@openwrt-bot
Copy link
Author

None:

Looking at what is output by the preprocessor things look ok at the tc/q_cake.c

void print_color_uint(enum output_type t, enum color_attr color, const char *key, const char *fmt, uint64_t value);
static inline void print_uint (enum output_type t, const char *key, const char *fmt, uint64_t value)
{ print_color_uint( t, COLOR_NONE, key, fmt, value); };

Need to see how 'print_colour_uint' expands.

@openwrt-bot
Copy link
Author

None:

Right, cracked it and it’s horrible!

print_uint is expanded thus: Note the type of value uint64_t

         void print_color_uint(enum output_type t, enum color_attr color, const char *key, const char *fmt, uint64_t value);

static inline void print_uint (enum output_type t, const char *key, const char *fmt, uint64_t value)
{ print_color_uint( t, COLOR_NONE, key, fmt, value); };

So far so good.

print_color_uint expands to:

        void print_color_uint(enum output_type t, enum color_attr color, const char *key, const char *fmt, uint64_t value)

{
if (((t & PRINT_JSON || t & PRINT_ANY) && _jw))
{ if (!key) jsonw_uint(_jw, value);
else jsonw_uint_field(_jw, key, value);
}
else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY)))
{ color_fprintf( (stdout) , color, fmt, value);
}
};

Again, no issue and we eventually call color_fprintf

int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
{
int ret = 0;
va_list args;

   va_start(args, fmt);

   if (!color_is_enabled || attr == COLOR_NONE) {
           ret = vfprintf(fp, fmt, args);
           goto end;
   }

Now, color_printf is a variable argument list function and as such is dependent upon being told the correct size of argument variables in the fmt variable. And that’s our problem, we’re passing a 64bit integer but telling the format string that it’s ‘int’…which I’m guessing can be variable sizes depending on architecture, as can the endianness.

If we instead do (in q_cake.c)

#include <inttypes.h>

print_uint(PRINT_ANY, "min_transport_size", " min/max transport layer size: %10" PRIu64, stnc->min_trnlen);

it works. This needs sanity checking by a clever person.

@openwrt-bot
Copy link
Author

None:

A suitable fix, also sent to upstream https://patchwork.ozlabs.org/patch/887411/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
flyspray
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@openwrt-bot