* [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
@ 2016-11-16 22:16 Roman Mashak
2016-11-17 18:42 ` David Miller
2016-11-17 21:02 ` Cong Wang
0 siblings, 2 replies; 6+ messages in thread
From: Roman Mashak @ 2016-11-16 22:16 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
Userland client should be able to read an event, and reflect it back to
the kernel, therefore it needs to extract complete set of netlink flags.
For example, this will allow "tc monitor" to distinguish Add and Replace
operations.
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_api.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2b2a797..8e93d4a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
it_chain = &tp->next)
- tfilter_notify(net, oskb, n, tp, 0, event, false);
+ tfilter_notify(net, oskb, n, tp, n->nlmsg_flags, event, false);
}
/* Select new prio value from the range, managed by kernel. */
@@ -430,7 +430,8 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
if (!skb)
return -ENOBUFS;
- if (tcf_fill_node(net, skb, tp, fh, portid, n->nlmsg_seq, 0, event) <= 0) {
+ if (tcf_fill_node(net, skb, tp, fh, portid, n->nlmsg_seq,
+ n->nlmsg_flags, event) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
2016-11-16 22:16 [PATCH net 1/1] net sched filters: pass netlink message flags in event notification Roman Mashak
@ 2016-11-17 18:42 ` David Miller
2016-11-17 21:02 ` Cong Wang
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-11-17 18:42 UTC (permalink / raw)
To: mrv; +Cc: netdev, jhs
From: Roman Mashak <mrv@mojatatu.com>
Date: Wed, 16 Nov 2016 17:16:10 -0500
> Userland client should be able to read an event, and reflect it back to
> the kernel, therefore it needs to extract complete set of netlink flags.
>
> For example, this will allow "tc monitor" to distinguish Add and Replace
> operations.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Looks good, applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
2016-11-16 22:16 [PATCH net 1/1] net sched filters: pass netlink message flags in event notification Roman Mashak
2016-11-17 18:42 ` David Miller
@ 2016-11-17 21:02 ` Cong Wang
2016-11-22 5:23 ` Cong Wang
1 sibling, 1 reply; 6+ messages in thread
From: Cong Wang @ 2016-11-17 21:02 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim
On Wed, Nov 16, 2016 at 2:16 PM, Roman Mashak <mrv@mojatatu.com> wrote:
> Userland client should be able to read an event, and reflect it back to
> the kernel, therefore it needs to extract complete set of netlink flags.
>
> For example, this will allow "tc monitor" to distinguish Add and Replace
> operations.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> net/sched/cls_api.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 2b2a797..8e93d4a 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
>
> for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
> it_chain = &tp->next)
> - tfilter_notify(net, oskb, n, tp, 0, event, false);
> + tfilter_notify(net, oskb, n, tp, n->nlmsg_flags, event, false);
I must miss something, why does it make sense to pass n->nlmsg_flags
as 'fh' to tfilter_notify()??
> }
>
> /* Select new prio value from the range, managed by kernel. */
> @@ -430,7 +430,8 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
> if (!skb)
> return -ENOBUFS;
>
> - if (tcf_fill_node(net, skb, tp, fh, portid, n->nlmsg_seq, 0, event) <= 0) {
> + if (tcf_fill_node(net, skb, tp, fh, portid, n->nlmsg_seq,
> + n->nlmsg_flags, event) <= 0) {
This part makes sense.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
2016-11-17 21:02 ` Cong Wang
@ 2016-11-22 5:23 ` Cong Wang
2016-11-22 9:28 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2016-11-22 5:23 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim
On Thu, Nov 17, 2016 at 1:02 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Nov 16, 2016 at 2:16 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>> Userland client should be able to read an event, and reflect it back to
>> the kernel, therefore it needs to extract complete set of netlink flags.
>>
>> For example, this will allow "tc monitor" to distinguish Add and Replace
>> operations.
>>
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>> ---
>> net/sched/cls_api.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index 2b2a797..8e93d4a 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
>>
>> for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
>> it_chain = &tp->next)
>> - tfilter_notify(net, oskb, n, tp, 0, event, false);
>> + tfilter_notify(net, oskb, n, tp, n->nlmsg_flags, event, false);
>
>
> I must miss something, why does it make sense to pass n->nlmsg_flags
> as 'fh' to tfilter_notify()??
Ping... Any response?
It still doesn't look correct to me. I will send a fix unless someone could
explain this.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
2016-11-22 5:23 ` Cong Wang
@ 2016-11-22 9:28 ` Daniel Borkmann
2016-11-22 16:02 ` Roman Mashak
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2016-11-22 9:28 UTC (permalink / raw)
To: Cong Wang, Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim
On 11/22/2016 06:23 AM, Cong Wang wrote:
> On Thu, Nov 17, 2016 at 1:02 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Wed, Nov 16, 2016 at 2:16 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>> Userland client should be able to read an event, and reflect it back to
>>> the kernel, therefore it needs to extract complete set of netlink flags.
>>>
>>> For example, this will allow "tc monitor" to distinguish Add and Replace
>>> operations.
>>>
>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>> ---
>>> net/sched/cls_api.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>>> index 2b2a797..8e93d4a 100644
>>> --- a/net/sched/cls_api.c
>>> +++ b/net/sched/cls_api.c
>>> @@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
>>>
>>> for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
>>> it_chain = &tp->next)
>>> - tfilter_notify(net, oskb, n, tp, 0, event, false);
>>> + tfilter_notify(net, oskb, n, tp, n->nlmsg_flags, event, false);
>>
>>
>> I must miss something, why does it make sense to pass n->nlmsg_flags
>> as 'fh' to tfilter_notify()??
>
> Ping... Any response?
>
> It still doesn't look correct to me. I will send a fix unless someone could
> explain this.
Sigh, I missed that this was applied already to -net (it certainly doesn't look
like -net material, but rather -net-next stuff) ... This definitely looks buggy
to me, the 0 as it was before was correct here (as it means we delete the whole
chain in this case).
If you could send a patch would be great. Thanks Cong!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/1] net sched filters: pass netlink message flags in event notification
2016-11-22 9:28 ` Daniel Borkmann
@ 2016-11-22 16:02 ` Roman Mashak
0 siblings, 0 replies; 6+ messages in thread
From: Roman Mashak @ 2016-11-22 16:02 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Cong Wang, David Miller, Linux Kernel Network Developers,
Jamal Hadi Salim
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 11/22/2016 06:23 AM, Cong Wang wrote:
>> On Thu, Nov 17, 2016 at 1:02 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Wed, Nov 16, 2016 at 2:16 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>> Userland client should be able to read an event, and reflect it back to
>>>> the kernel, therefore it needs to extract complete set of netlink flags.
>>>>
>>>> For example, this will allow "tc monitor" to distinguish Add and Replace
>>>> operations.
>>>>
>>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>>> ---
>>>> net/sched/cls_api.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>>>> index 2b2a797..8e93d4a 100644
>>>> --- a/net/sched/cls_api.c
>>>> +++ b/net/sched/cls_api.c
>>>> @@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
>>>>
>>>> for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
>>>> it_chain = &tp->next)
>>>> - tfilter_notify(net, oskb, n, tp, 0, event, false);
>>>> + tfilter_notify(net, oskb, n, tp, n->nlmsg_flags, event, false);
>>>
>>>
>>> I must miss something, why does it make sense to pass n->nlmsg_flags
>>> as 'fh' to tfilter_notify()??
>>
>> Ping... Any response?
>>
>> It still doesn't look correct to me. I will send a fix unless someone could
>> explain this.
>
> Sigh, I missed that this was applied already to -net (it certainly doesn't look
> like -net material, but rather -net-next stuff) ... This definitely looks buggy
> to me, the 0 as it was before was correct here (as it means we delete the whole
> chain in this case).
>
> If you could send a patch would be great. Thanks Cong!
Cong/Daniel, sorry for late response, I was distracted.
I apologize, I will send a fix today.
--
Roman Mashak
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-22 16:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-16 22:16 [PATCH net 1/1] net sched filters: pass netlink message flags in event notification Roman Mashak
2016-11-17 18:42 ` David Miller
2016-11-17 21:02 ` Cong Wang
2016-11-22 5:23 ` Cong Wang
2016-11-22 9:28 ` Daniel Borkmann
2016-11-22 16:02 ` Roman Mashak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).