* [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
@ 2017-07-04 0:14 McCabe, Robert J
2017-07-04 0:14 ` [PATCH 1/1] tc: custom qdisc pkt size translation table McCabe, Robert J
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: McCabe, Robert J @ 2017-07-04 0:14 UTC (permalink / raw)
To: netdev; +Cc: robert.mccabe, McCabe, Robert J, McCabe
This is to support user-space modification of the qdisc stab.
Signed-off-by: McCabe, Robert J <Robert.McCabe@rockwellcollins.com>
---
include/uapi/linux/pkt_sched.h | 1 +
net/sched/sch_api.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 099bf55..289bb81 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -82,6 +82,7 @@ enum tc_link_layer {
TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
TC_LINKLAYER_ETHERNET,
TC_LINKLAYER_ATM,
+ TC_LINKLAYER_CUSTOM,
};
#define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 43b94c7..174a925 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -533,6 +533,8 @@ static int qdisc_dump_stab(struct sk_buff *skb, struct qdisc_size_table *stab)
goto nla_put_failure;
if (nla_put(skb, TCA_STAB_BASE, sizeof(stab->szopts), &stab->szopts))
goto nla_put_failure;
+ if (nla_put(skb, TCA_STAB_DATA, sizeof(stab->szopts)*sizeof(u16), &stab->data))
+ goto nla_put_failure;
nla_nest_end(skb, nest);
return skb->len;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/1] tc: custom qdisc pkt size translation table
2017-07-04 0:14 [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type McCabe, Robert J
@ 2017-07-04 0:14 ` McCabe, Robert J
2017-07-04 5:48 ` [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type Jiri Pirko
2017-07-06 9:03 ` David Laight
2 siblings, 0 replies; 9+ messages in thread
From: McCabe, Robert J @ 2017-07-04 0:14 UTC (permalink / raw)
To: netdev; +Cc: robert.mccabe, McCabe, Robert J, McCabe
Added the "custom" linklayer qdisc stab option.
Allows the user to specify the pkt size translation
parameters from stdin.
Example:
tc qdisc add ... stab tsize 8 linklayer custom htb
Custom size table:
InputSizeStart -> IntputSizeEnd: OutputSize
0 -> 511 : 600
512 -> 1023 : 1200
1024 -> 1535 : 1800
1536 -> 2047 : 2400
2048 -> 2559 : 3000
blah
Signed-off-by: McCabe, Robert J <Robert.McCabe@rockwellcollins.com>
---
include/linux/pkt_sched.h | 1 +
tc/tc_core.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
tc/tc_core.h | 2 +-
tc/tc_stab.c | 20 +++++++++-----------
tc/tc_util.c | 5 +++++
5 files changed, 62 insertions(+), 12 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 099bf55..289bb81 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -82,6 +82,7 @@ enum tc_link_layer {
TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
TC_LINKLAYER_ETHERNET,
TC_LINKLAYER_ATM,
+ TC_LINKLAYER_CUSTOM,
};
#define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */
diff --git a/tc/tc_core.c b/tc/tc_core.c
index 821b741..167f4d7 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -21,6 +21,7 @@
#include <netinet/in.h>
#include <arpa/inet.h>
#include <string.h>
+#include <assert.h>
#include "tc_core.h"
#include <linux/atm.h>
@@ -28,6 +29,13 @@
static double tick_in_usec = 1;
static double clock_factor = 1;
+struct size_table_entry {
+ unsigned int input_size_boundary_start;
+ unsigned int output_size_bytes;
+};
+
+static struct size_table_entry* custom_size_table = NULL;
+static int num_size_table_entries = 0;
int tc_core_time2big(unsigned int time)
{
__u64 t = time;
@@ -89,6 +97,20 @@ static unsigned int tc_align_to_atm(unsigned int size)
return linksize;
}
+static unsigned int tc_align_to_custom(unsigned int size)
+{
+ int i;
+
+ assert(custom_size_table != NULL);
+ for(i = num_size_table_entries -1; i >= 0 ; --i) {
+ if(custom_size_table[i].input_size_boundary_start < size) {
+ /* found it */
+ return custom_size_table[i].output_size_bytes;
+ }
+ }
+ return 0;
+}
+
static unsigned int tc_adjust_size(unsigned int sz, unsigned int mpu, enum link_layer linklayer)
{
if (sz < mpu)
@@ -97,6 +119,8 @@ static unsigned int tc_adjust_size(unsigned int sz, unsigned int mpu, enum link_
switch (linklayer) {
case LINKLAYER_ATM:
return tc_align_to_atm(sz);
+ case LINKLAYER_CUSTOM:
+ return tc_align_to_custom(sz);
case LINKLAYER_ETHERNET:
default:
/* No size adjustments on Ethernet */
@@ -185,6 +209,24 @@ int tc_calc_size_table(struct tc_sizespec *s, __u16 **stab)
if (!*stab)
return -1;
+ if(LINKLAYER_CUSTOM == linklayer) {
+ custom_size_table = malloc(sizeof(struct size_table_entry)* s->tsize);
+ if(!custom_size_table)
+ return -1;
+ num_size_table_entries = s->tsize;
+
+ printf("Custom size table:\n");
+ printf("InputSizeStart -> IntputSizeEnd: OutputSize\n");
+ for(i = 0; i <= s->tsize - 1; ++i) {
+ printf("%-14d -> %-13d: ", i << s->cell_log, ((i+1) << s->cell_log) - 1);
+ if(!scanf("%u", &custom_size_table[i].output_size_bytes)) {
+ fprintf(stderr, "Invalid custom stab table entry!\n");
+ return -1;
+ }
+ custom_size_table[i].input_size_boundary_start = i << s->cell_log;
+ }
+ }
+
again:
for (i = s->tsize - 1; i >= 0; i--) {
sz = tc_adjust_size((i + 1) << s->cell_log, s->mpu, linklayer);
@@ -196,6 +238,10 @@ again:
}
s->cell_align = -1; /* Due to the sz calc */
+ if(custom_size_table) {
+ free(custom_size_table);
+ num_size_table_entries = 0;
+ }
return 0;
}
diff --git a/tc/tc_core.h b/tc/tc_core.h
index 8a63b79..8e97222 100644
--- a/tc/tc_core.h
+++ b/tc/tc_core.h
@@ -10,9 +10,9 @@ enum link_layer {
LINKLAYER_UNSPEC,
LINKLAYER_ETHERNET,
LINKLAYER_ATM,
+ LINKLAYER_CUSTOM,
};
-
int tc_core_time2big(unsigned time);
unsigned tc_core_time2tick(unsigned time);
unsigned tc_core_tick2time(unsigned tick);
diff --git a/tc/tc_stab.c b/tc/tc_stab.c
index 1a0a3e3..a468a70 100644
--- a/tc/tc_stab.c
+++ b/tc/tc_stab.c
@@ -37,7 +37,9 @@ static void stab_help(void)
" tsize : how many slots should size table have {512}\n"
" mpu : minimum packet size used in rate computations\n"
" overhead : per-packet size overhead used in rate computations\n"
- " linklayer : adapting to a linklayer e.g. atm\n"
+ " linklayer : adapting to a linklayer e.g. ethernet, atm or custom\n"
+ " a \"custom\" linklayer reads size table entries from\n"
+ " stdin\n"
"Example: ... stab overhead 20 linklayer atm\n");
}
@@ -107,13 +109,13 @@ int parse_size_table(int *argcp, char ***argvp, struct tc_sizespec *sp)
void print_size_table(FILE *fp, const char *prefix, struct rtattr *rta)
{
struct rtattr *tb[TCA_STAB_MAX + 1];
+ struct tc_sizespec s = {0};
SPRINT_BUF(b1);
parse_rtattr_nested(tb, TCA_STAB_MAX, rta);
if (tb[TCA_STAB_BASE]) {
- struct tc_sizespec s = {0};
memcpy(&s, RTA_DATA(tb[TCA_STAB_BASE]),
MIN(RTA_PAYLOAD(tb[TCA_STAB_BASE]), sizeof(s)));
@@ -132,19 +134,15 @@ void print_size_table(FILE *fp, const char *prefix, struct rtattr *rta)
fprintf(fp, "tsize %u ", s.tsize);
}
-#if 0
if (tb[TCA_STAB_DATA]) {
unsigned int i, j, dlen;
__u16 *data = RTA_DATA(tb[TCA_STAB_DATA]);
- dlen = RTA_PAYLOAD(tb[TCA_STAB_DATA]) / sizeof(__u16);
-
fprintf(fp, "\n%sstab data:", prefix);
- for (i = 0; i < dlen/12; i++) {
- fprintf(fp, "\n%s %3u:", prefix, i * 12);
- for (j = 0; i * 12 + j < dlen; j++)
- fprintf(fp, " %05x", data[i * 12 + j]);
- }
+ fprintf(fp, "\n%sInputSizeStart -> IntputSizeEnd: Output Size\n", prefix);
+ for(i = 0; i <= s.tsize - 1; ++i) {
+ fprintf(fp, "%s%-14d -> %-13d: %d\n", prefix, i << s.cell_log,
+ ((i+1) << s.cell_log) - 1, data[i]);
+ }
}
-#endif
}
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 3710468..62019c1 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -651,6 +651,8 @@ int get_linklayer(unsigned int *val, const char *arg)
res = LINKLAYER_ATM;
else if (matches(arg, "adsl") == 0)
res = LINKLAYER_ATM;
+ else if (matches(arg, "custom") == 0)
+ res = LINKLAYER_CUSTOM;
else
return -1; /* Indicate error */
@@ -670,6 +672,9 @@ void print_linklayer(char *buf, int len, unsigned int linklayer)
case LINKLAYER_ATM:
snprintf(buf, len, "%s", "atm");
return;
+ case LINKLAYER_CUSTOM:
+ snprintf(buf, len, "%s", "custom");
+ return;
default:
snprintf(buf, len, "%s", "unknown");
return;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 0:14 [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type McCabe, Robert J
2017-07-04 0:14 ` [PATCH 1/1] tc: custom qdisc pkt size translation table McCabe, Robert J
@ 2017-07-04 5:48 ` Jiri Pirko
2017-07-04 14:49 ` Robert McCabe
2017-07-06 9:03 ` David Laight
2 siblings, 1 reply; 9+ messages in thread
From: Jiri Pirko @ 2017-07-04 5:48 UTC (permalink / raw)
To: McCabe, Robert J; +Cc: netdev, McCabe
Tue, Jul 04, 2017 at 02:14:25AM CEST, Robert.McCabe@rockwellcollins.com wrote:
>This is to support user-space modification of the qdisc stab.
>
>Signed-off-by: McCabe, Robert J <Robert.McCabe@rockwellcollins.com>
>---
> include/uapi/linux/pkt_sched.h | 1 +
> net/sched/sch_api.c | 2 ++
> 2 files changed, 3 insertions(+)
>
>diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
>index 099bf55..289bb81 100644
>--- a/include/uapi/linux/pkt_sched.h
>+++ b/include/uapi/linux/pkt_sched.h
>@@ -82,6 +82,7 @@ enum tc_link_layer {
> TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
> TC_LINKLAYER_ETHERNET,
> TC_LINKLAYER_ATM,
>+ TC_LINKLAYER_CUSTOM,
> };
> #define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */
>
>diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
>index 43b94c7..174a925 100644
>--- a/net/sched/sch_api.c
>+++ b/net/sched/sch_api.c
>@@ -533,6 +533,8 @@ static int qdisc_dump_stab(struct sk_buff *skb, struct qdisc_size_table *stab)
> goto nla_put_failure;
> if (nla_put(skb, TCA_STAB_BASE, sizeof(stab->szopts), &stab->szopts))
> goto nla_put_failure;
>+ if (nla_put(skb, TCA_STAB_DATA, sizeof(stab->szopts)*sizeof(u16), &stab->data))
>+ goto nla_put_failure;
You dump stab->data to user. How is this related to TC_LINKLAYER_CUSTOM
and howcome this "is to support user-space modification of the qdisc
stab" as your description says? I'm confused...
> nla_nest_end(skb, nest);
>
> return skb->len;
>--
>2.7.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 5:48 ` [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type Jiri Pirko
@ 2017-07-04 14:49 ` Robert McCabe
2017-07-04 15:34 ` Jiri Pirko
0 siblings, 1 reply; 9+ messages in thread
From: Robert McCabe @ 2017-07-04 14:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, McCabe
> You dump stab->data to user. How is this related to TC_LINKLAYER_CUSTOM
> and howcome this "is to support user-space modification of the qdisc
> stab" as your description says? I'm confused...
I have a related patch for iproute2 where the "custom" link-layer is
used to allow
user modification of the qdisc stab. Dumping stab->data to the user
is used in the
iproute2 patch to show the current size translation table.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 14:49 ` Robert McCabe
@ 2017-07-04 15:34 ` Jiri Pirko
2017-07-04 15:40 ` Jiri Pirko
0 siblings, 1 reply; 9+ messages in thread
From: Jiri Pirko @ 2017-07-04 15:34 UTC (permalink / raw)
To: Robert McCabe; +Cc: netdev, McCabe
Tue, Jul 04, 2017 at 04:49:24PM CEST, robert.mccabe@rockwellcollins.com wrote:
>> You dump stab->data to user. How is this related to TC_LINKLAYER_CUSTOM
>> and howcome this "is to support user-space modification of the qdisc
>> stab" as your description says? I'm confused...
>
>I have a related patch for iproute2 where the "custom" link-layer is
>used to allow
>user modification of the qdisc stab. Dumping stab->data to the user
>is used in the
>iproute2 patch to show the current size translation table.
Even so, the kernel patch you sent does not make any sense. Introduces
TC_LINKLAYER_CUSTOM but does not use it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 15:34 ` Jiri Pirko
@ 2017-07-04 15:40 ` Jiri Pirko
2017-07-04 17:12 ` Robert McCabe
0 siblings, 1 reply; 9+ messages in thread
From: Jiri Pirko @ 2017-07-04 15:40 UTC (permalink / raw)
To: Robert McCabe; +Cc: netdev, McCabe
Tue, Jul 04, 2017 at 05:34:50PM CEST, jiri@resnulli.us wrote:
>Tue, Jul 04, 2017 at 04:49:24PM CEST, robert.mccabe@rockwellcollins.com wrote:
>>> You dump stab->data to user. How is this related to TC_LINKLAYER_CUSTOM
>>> and howcome this "is to support user-space modification of the qdisc
>>> stab" as your description says? I'm confused...
>>
>>I have a related patch for iproute2 where the "custom" link-layer is
>>used to allow
>>user modification of the qdisc stab. Dumping stab->data to the user
>>is used in the
>>iproute2 patch to show the current size translation table.
>
>Even so, the kernel patch you sent does not make any sense. Introduces
>TC_LINKLAYER_CUSTOM but does not use it.
Also, please make you email working. Says to me:
** Address not found **
Your message wasn't delivered to McCabe@rockwellcollins.com because the address couldn't be found. Check for typos or unnecessary spaces and try again.
This is annoying.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 15:40 ` Jiri Pirko
@ 2017-07-04 17:12 ` Robert McCabe
2017-07-05 7:01 ` Jiri Pirko
0 siblings, 1 reply; 9+ messages in thread
From: Robert McCabe @ 2017-07-04 17:12 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev
>>Even so, the kernel patch you sent does not make any sense. Introduces
>>TC_LINKLAYER_CUSTOM but does not use it.
I needed to add this to the tc_link_layer enum in my iproute2 patch
(https://patchwork.ozlabs.org/patch/784165/)
enum tc_link_layer {
TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
TC_LINKLAYER_ETHERNET,
TC_LINKLAYER_ATM,
+ TC_LINKLAYER_CUSTOM,
};
I originally wasn't aware that this file was shared with the kernel --
my original patch received the comment from Eric Dumazet
You can not do this : This file is coming from the kernel
( include/uapi/linux/pkt_sched.h )
So I opted to make the corresponding change to the pkt_sched.h file in
the kernel.
But looking at it further, I am confused as to why this tc_link_layer even needs
to exist in the kernel anyway, It is only used in net/sched for calculating the
size (via stab) and rate (via rtab) translations; however, these
translations are
already specified with in the form of the stab and rtab themselves (these are
calculated in userspace).
I think -- in a perfect world -- the tc_link_layer (and the associated
tc_ratespec.linklayer and tc_sizespec.linklayer) should be removed
from the kernel,
but I'm not sure of the compatibility implications of this ...
Do you have any suggestions?
>
> Also, please make you email working. Says to me:
>
> ** Address not found **
>
> Your message wasn't delivered to McCabe@rockwellcollins.com because the address couldn't be found. Check for typos or unnecessary spaces and try again.
>
> This is annoying.
I think I fixed it -- was mis-configuration in my .gitconfig (sorry,
I'm still new at this).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 17:12 ` Robert McCabe
@ 2017-07-05 7:01 ` Jiri Pirko
0 siblings, 0 replies; 9+ messages in thread
From: Jiri Pirko @ 2017-07-05 7:01 UTC (permalink / raw)
To: Robert McCabe; +Cc: netdev
Tue, Jul 04, 2017 at 07:12:47PM CEST, robert.mccabe@rockwellcollins.com wrote:
>>>Even so, the kernel patch you sent does not make any sense. Introduces
>>>TC_LINKLAYER_CUSTOM but does not use it.
>
>I needed to add this to the tc_link_layer enum in my iproute2 patch
>(https://patchwork.ozlabs.org/patch/784165/)
>
>enum tc_link_layer {
> TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
> TC_LINKLAYER_ETHERNET,
> TC_LINKLAYER_ATM,
> + TC_LINKLAYER_CUSTOM,
> };
>
>I originally wasn't aware that this file was shared with the kernel --
>my original patch received the comment from Eric Dumazet
>
> You can not do this : This file is coming from the kernel
> ( include/uapi/linux/pkt_sched.h )
Correct.
>
>So I opted to make the corresponding change to the pkt_sched.h file in
>the kernel.
I'm not sure how should I put this, but you are adding enum value and
you are not using it in kernel! That is wrong. Whenever you add new
value to UAPI, it has to have a point. Please see:
$ git grep TC_LINKLAYER_
>But looking at it further, I am confused as to why this tc_link_layer even needs
>to exist in the kernel anyway, It is only used in net/sched for calculating the
>size (via stab) and rate (via rtab) translations; however, these
>translations are
>already specified with in the form of the stab and rtab themselves (these are
>calculated in userspace).
>
>I think -- in a perfect world -- the tc_link_layer (and the associated
>tc_ratespec.linklayer and tc_sizespec.linklayer) should be removed
>from the kernel,
>but I'm not sure of the compatibility implications of this ...
>Do you have any suggestions?
>
>>
>> Also, please make you email working. Says to me:
>>
>> ** Address not found **
>>
>> Your message wasn't delivered to McCabe@rockwellcollins.com because the address couldn't be found. Check for typos or unnecessary spaces and try again.
>>
>> This is annoying.
>
>I think I fixed it -- was mis-configuration in my .gitconfig (sorry,
>I'm still new at this).
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type
2017-07-04 0:14 [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type McCabe, Robert J
2017-07-04 0:14 ` [PATCH 1/1] tc: custom qdisc pkt size translation table McCabe, Robert J
2017-07-04 5:48 ` [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type Jiri Pirko
@ 2017-07-06 9:03 ` David Laight
2 siblings, 0 replies; 9+ messages in thread
From: David Laight @ 2017-07-06 9:03 UTC (permalink / raw)
To: 'McCabe, Robert J', netdev; +Cc: McCabe
From: McCabe, Robert J
> Sent: 04 July 2017 01:14
> + if (nla_put(skb, TCA_STAB_DATA, sizeof(stab->szopts)*sizeof(u16), &stab->data))
> + goto nla_put_failure;
Multiplying sizeof(a) by sizeof(b) really doesn't look right at all.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-06 9:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-04 0:14 [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type McCabe, Robert J
2017-07-04 0:14 ` [PATCH 1/1] tc: custom qdisc pkt size translation table McCabe, Robert J
2017-07-04 5:48 ` [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type Jiri Pirko
2017-07-04 14:49 ` Robert McCabe
2017-07-04 15:34 ` Jiri Pirko
2017-07-04 15:40 ` Jiri Pirko
2017-07-04 17:12 ` Robert McCabe
2017-07-05 7:01 ` Jiri Pirko
2017-07-06 9:03 ` David Laight
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.