Re: [patch net-next v8 08/14] net: sched: add rt netlink message type for block get

[Jiri Pirko <jiri@resnulli.us>]

Mon, Jan 15, 2018 at 06:44:41PM CET, dsahern@gmail.com wrote:
>On 1/15/18 10:27 AM, Jiri Pirko wrote:
>> Mon, Jan 15, 2018 at 06:21:44PM CET, dsahern@gmail.com wrote:
>>> On 1/15/18 10:08 AM, David Ahern wrote:
>>>> On 1/15/18 10:03 AM, Jiri Pirko wrote:
>>>>> Mon, Jan 15, 2018 at 05:56:31PM CET, dsahern@gmail.com wrote:
>>>>>> On 1/12/18 8:46 AM, Jiri Pirko wrote:
>>>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>>
>>>>>> Why can't this be done with RTM_GETQDISC?
>>>>>
>>>>> I don't follow. Could you please describe a bit more what do you think?
>>>>
>>>> Why are you adding RTM_{NEW,GET,DEL}BLOCK? Can't you get the same
>>>> information using RTM_GETQDISC and updating it to check for the
>>>> 'tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK' path
>> 
>> I might, but it bould be an ugly hack. I would use cmd that is used to
>> manipulate qdisc to some entirely different purpose. That does not make
>> any sense to me :(
>> 
>> 
>> 
>>>>
>>>
>>> The above question is because a user specifies a shared block in a
>>> 'qdisc add'.
>> 
>> Qdisc and block is a different entity
>> 
>> 
>>>
>>> Alternatively, what about RTM_GETTFILTER? You already update
>>> tc_ctl_tfilter to check for TCM_IFINDEX_MAGIC_BLOCK
>> 
>> The object is still filter! Only the handle is different. You cannot
>> compare that, sorry.
>> 
>> 
>>>
>>> My main question is why can't existing RTM_ commands be used?
>
>What I am struggling with is the idea that you need a new set of RTM_
>commands to see if a block exists or to get notifications of a change to
>a block, but you don't need that API to create or modify the blocks.

There is no modify. Create and destroy is done implicitly. That is the
same as for the chains.

I was thinking about how to get info if block exists or not. One way was
new set of rtms that would be also usable for notifications. If we
don't need notifications, it can be done by listing all qdiscs and see
if they don't reference the block. That was not originally possible
because the block info was per-qdisc. Now, per Roopa's suggestion it is
generic.

Okay. I will use qdisc dump for checking block existence. I guess that
block create/destroy notifications are not that useful anyway. We don't
have them for chains either.

Thanks.