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@mojatatu.com wrote:

quoted

On 18-01-06 03:43 PM, Jiri Pirko wrote:

quoted

quoted

@@ -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