On 17-04-20 07:35 AM, Jiri Pirko wrote:
Thu, Apr 20, 2017 at 12:42:55PM CEST, j...@mojatatu.com wrote:
On 17-04-19 12:17 PM, Jiri Pirko wrote:
Ha. So the current code is wrong, I see it now. Following is needed:
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 82b1d48..c432b22 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1005,7 +1005,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct
nlmsghdr *n,
!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
- ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL,
+ ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, NULL,
extack);
if (ret < 0)
return ret;
You confused me squashing the change to this patch.
Ok, so the name could be:
TCA_ACTS_*
?
I believe it is crucial to figure this out right now. TC UAPI is in deep
mess already, no need to push it even more.
I could change that name. Look at the last series i sent, add any extra
comments to that and i will get to it by tomorrow.
They are not the same issue Jiri. We have used bitmasks fine on netlink
Howcome they are not the same? I'm pretty certain they are.
They are not Jiri. I challenge you to point me to one netlink bitmask
representation used that has some bits that were not being used that
someone or some compiler is going to set some random values on.
Typically flags are u64/32/16
message for a millenia. Nobody sets garbage on a bitmask they are not
supposed to touch. The struct padding thing is a shame the way it
turned out - now netlink can no longer have a claim to be a (good)
wire protocol.
Other thing: I know you feel strongly about this but I dont agree that
everything has to be a TLV and in no way, as a matter of principle, you are
going to convince me (even when the cows come home) that I have to
use 64 bits to carry a single bit just so I can use TLVs.
Then I guess we have to agree to disagree. I believe that your approach
is wrong and will break users in future when you add even a single flag.
Argument that "we are doing this for ages-therefore it is correct" has
simply 0 value.
"doing these for 80 years" is not the main driving point.
I appreciate that data structures - even when fields are
annotated as "pads" there is no guarantee when i allocate one
the pads will be zeroes.
Bitmasks are atomic in nature and therefore used in whole - not complex
like structs. Thats the difference. I dont define this u32 bitmap that
has 8 bits that are "pads" because that concept doesnt make sense for
atomic fields.
This implies that 6 months down the road I dont regret and say "shit, I
need 2 more bits for flagbits but some idiot or compiler was setting
those fields so i cant use them now because the kernel wont tell the
difference between what s/he is doing and real values".
Allocations of data structures I can understand, unless we enforce
always reset to zeroes all the struct members as part of creation.
But lets agree to disagree if you are still not convinced.
On your "everything must be a TLV". Caveat is: there is a balance
between performance and flexibility. I should be able to pack aligned
complex data structures in a TLV for performance reasons.
One lesson is: in the future we can make these extra service header like
tcamsg very small and always use all their fields instead of defining
pads.
cheers,
jamal
