Mon, Jun 12, 2017 at 01:16:05PM CEST, jhs@mojatatu.com wrote:

On 17-06-11 10:13 AM, Jiri Pirko wrote:

quoted

Sun, Jun 11, 2017 at 01:53:45PM CEST, jhs@mojatatu.com wrote:

[..]

quoted

quoted

@@ -1168,14 +1185,24 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)

	if (a_o == NULL)
		return 0;

+	if (tb[TCA_ROOT_FLAGS])
+		nla_memcpy(&select_flags, tb[TCA_ROOT_FLAGS],
+			   sizeof(select_flags));

Please introduce a helper for this attr type in patch 1:

u32 select_flags;

select_flags = nla_get_flag_bits_values(tb[TCA_ROOT_FLAGS])

This also is not useful.
It happens to be ok for this use case but not for the
general case. i.e.
We need to get the whole struct not just the values
and use the selector to pick what bits are affected.
Example if bit X is set to 1 in selector and bit X in value
is 0, then we set the kernel's bit X to 0.

Sure, have another helper for selector then.
Or, you can have:

	struct nla_flag_bits *fb;
	fb = nla_get_flag_bits(tb[TCA_ROOT_FLAGS]);

Or all 3 helpers. My point is, it is a specific netlink attribute with
specific format, it should have get/put helpers.