Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type

[Robert McCabe]

>>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 mcc...@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).

Re: [PATCH 1/1] net sched: Added the TC_LINKLAYER_CUSTOM linklayer type

[Robert 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.

Re: [PATCH 1/1] tc: custom qdisc pkt size translation table

[Robert McCabe]

Yeah, sorry didn't even think about that.
I guess my first question would be is there another way via the
iproute2 project where a user could
configure the stab->data pkt size translation table used in the
__qdisc_calculate_pkt_len method
in the kernel source (net/sched/sched_api.c)?

Also, let's say I went ahead and made the added TC_LINK_LAYER_CUSTOM
to the include/uapi/linux/pkt_sched.h
file in the kernel source ... would I also need to make the same
change in include/uapi/linux/pkt_sched.h in
the iproute2 source?

Do you recommend an alternative (more elegant) approach to what I'm
trying to accomplish?


On Tue, Jun 27, 2017 at 11:55 AM, Eric Dumazet  wrote:
> On Tue, 2017-06-27 at 11:29 -0500, McCabe, Robert J wrote:
>> Added the "custom" linklayer qdisc stab option.
>> This 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: Output Pkt Size
>>0 - 255: 400
>>256 - 511: 800
>>512 - 767: 1200
>>768 - 1023: 1600
>>1024 - 1279: 2000
>>1280 - 1535: 2400
>>1536 - 1791: 2800
>>1792 - 2047: 3200
>>
>> Signed-off-by: McCabe, Robert J 
>> ---
>>  include/linux/pkt_sched.h |  1 +
>>  tc/tc_core.c  | 51 
>> +++
>>  tc/tc_core.h  |  2 +-
>>  tc/tc_stab.c  |  4 +++-
>>  tc/tc_util.c  |  5 +
>>  5 files changed, 61 insertions(+), 2 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 */
>>
>
>
> You can not do this : This file is coming from the kernel
> ( include/uapi/linux/pkt_sched.h )
>
> Since your patch is user space only, you need to find another way ?
>
>
>