netdev - Re: [patch net-next v6 06/11] net: sched: use block index as a handle instead of qdisc when block is shared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <48492de7-60d7-2bda-0a4d-b1d8f18cfd10@mojatatu.com>
Date:   Sun, 7 Jan 2018 09:28:13 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        xiyou.wangcong@...il.com, mlxsw@...lanox.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        michael.chan@...adcom.com, ganeshgr@...lsio.com,
        saeedm@...lanox.com, matanb@...lanox.com, leonro@...lanox.com,
        idosch@...lanox.com, jakub.kicinski@...ronome.com,
        simon.horman@...ronome.com, pieter.jansenvanvuuren@...ronome.com,
        john.hurley@...ronome.com, alexander.h.duyck@...el.com,
        ogerlitz@...lanox.com, john.fastabend@...il.com,
        daniel@...earbox.net, dsahern@...il.com
Subject: Re: [patch net-next v6 06/11] net: sched: use block index as a handle
 instead of qdisc when block is shared

On 18-01-07 08:46 AM, Jiri Pirko wrote:
> Sun, Jan 07, 2018 at 02:11:19PM CET, jhs@...atatu.com wrote:
>> On 18-01-06 03:43 PM, Jiri Pirko wrote:
>>
>>
>>>
>>>> @@ -886,8 +887,13 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
>>>> 	tcm->tcm_family = AF_UNSPEC;
>>>> 	tcm->tcm__pad1 = 0;
>>>> 	tcm->tcm__pad2 = 0;
>>>> -	tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
>>>> -	tcm->tcm_parent = parent;
>>>> +	if (q) {
>>>> +		tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
>>>> +		tcm->tcm_parent = parent;
>>>> +	} else {
>>>> +		tcm->tcm_ifindex = 0; /* block index is stored in parent */
>>>> +		tcm->tcm_parent = block->index;
>>>> +	}
>>>
>>> Please guys, please look at this reuse (also on clt side). I would like
>>> you to double-check this reuse of existing API for balock_index carrying
>>> purpose. I believe it's UAPI safe. But please, check it out carefully.
>>>
>>
>>
>> Should not break any ABI/UAPI AFAIK. Maybe go for a negative ifindex
>> (not sure if zero means something speacial to someone).
> 
> Like -1 means parent is block_index?
> 

Yes.

> Why would 0 mean something special? Could you point to a code that
> suggests it?
> 

I cant point to any such code, it is just the ifindex is an int.
And the negative space looks like less likely someone would think
of using for signalling (0xFFFFFFFF as an example).
tcpdump -i any probably assumes soem weird ifindex (havent looked
at the code).

In any case, 0 is fine too.

cheers,
jamal