* [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
@ 2017-10-26 21:40 Roman Mashak
2017-10-27 21:55 ` Cong Wang
0 siblings, 1 reply; 9+ messages in thread
From: Roman Mashak @ 2017-10-26 21:40 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, xiyou.wangcong, jiri, 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
qdisc operations.
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/sch_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index a9ac912..e3e29be 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -859,7 +859,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
}
if (new && !tc_qdisc_dump_ignore(new, false)) {
if (tc_fill_qdisc(skb, new, clid, portid, n->nlmsg_seq,
- old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
+ n->nlmsg_flags, RTM_NEWQDISC) < 0)
goto err_out;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-26 21:40 [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification Roman Mashak
@ 2017-10-27 21:55 ` Cong Wang
2017-10-29 3:36 ` Roman Mashak
0 siblings, 1 reply; 9+ messages in thread
From: Cong Wang @ 2017-10-27 21:55 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
On Thu, Oct 26, 2017 at 2:40 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
> qdisc operations.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> net/sched/sch_api.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index a9ac912..e3e29be 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -859,7 +859,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
> }
> if (new && !tc_qdisc_dump_ignore(new, false)) {
> if (tc_fill_qdisc(skb, new, clid, portid, n->nlmsg_seq,
> - old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
> + n->nlmsg_flags, RTM_NEWQDISC) < 0)
Don't you want to change the other tc_fill_qdisc() in the same function
too? ;)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-27 21:55 ` Cong Wang
@ 2017-10-29 3:36 ` Roman Mashak
2017-10-30 16:23 ` Cong Wang
0 siblings, 1 reply; 9+ messages in thread
From: Roman Mashak @ 2017-10-29 3:36 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Thu, Oct 26, 2017 at 2:40 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
>> qdisc operations.
>>
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>> net/sched/sch_api.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
>> index a9ac912..e3e29be 100644
>> --- a/net/sched/sch_api.c
>> +++ b/net/sched/sch_api.c
>> @@ -859,7 +859,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
>> }
>> if (new && !tc_qdisc_dump_ignore(new, false)) {
>> if (tc_fill_qdisc(skb, new, clid, portid, n->nlmsg_seq,
>> - old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
>> + n->nlmsg_flags, RTM_NEWQDISC) < 0)
>
>
> Don't you want to change the other tc_fill_qdisc() in the same function
> too? ;)
The other tc_fill_qdisc generates RTM_DELQDISC event, in that case
netlink flags for user don't matter.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-29 3:36 ` Roman Mashak
@ 2017-10-30 16:23 ` Cong Wang
2017-10-30 18:07 ` Roman Mashak
0 siblings, 1 reply; 9+ messages in thread
From: Cong Wang @ 2017-10-30 16:23 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
> Cong Wang <xiyou.wangcong@gmail.com> writes:
>
>> On Thu, Oct 26, 2017 at 2:40 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
>>> qdisc operations.
>>>
>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>> ---
>>> net/sched/sch_api.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
>>> index a9ac912..e3e29be 100644
>>> --- a/net/sched/sch_api.c
>>> +++ b/net/sched/sch_api.c
>>> @@ -859,7 +859,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
>>> }
>>> if (new && !tc_qdisc_dump_ignore(new, false)) {
>>> if (tc_fill_qdisc(skb, new, clid, portid, n->nlmsg_seq,
>>> - old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
>>> + n->nlmsg_flags, RTM_NEWQDISC) < 0)
>>
>>
>> Don't you want to change the other tc_fill_qdisc() in the same function
>> too? ;)
>
> The other tc_fill_qdisc generates RTM_DELQDISC event, in that case
> netlink flags for user don't matter.
Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
determine it is replacement, no?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-30 16:23 ` Cong Wang
@ 2017-10-30 18:07 ` Roman Mashak
2017-10-30 19:23 ` Cong Wang
0 siblings, 1 reply; 9+ messages in thread
From: Roman Mashak @ 2017-10-30 18:07 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>
>>> On Thu, Oct 26, 2017 at 2:40 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
>>>> qdisc operations.
>>>>
>>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>>> ---
>>>> net/sched/sch_api.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
>>>> index a9ac912..e3e29be 100644
>>>> --- a/net/sched/sch_api.c
>>>> +++ b/net/sched/sch_api.c
>>>> @@ -859,7 +859,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
>>>> }
>>>> if (new && !tc_qdisc_dump_ignore(new, false)) {
>>>> if (tc_fill_qdisc(skb, new, clid, portid, n->nlmsg_seq,
>>>> - old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
>>>> + n->nlmsg_flags, RTM_NEWQDISC) < 0)
>>>
>>>
>>> Don't you want to change the other tc_fill_qdisc() in the same function
>>> too? ;)
>>
>> The other tc_fill_qdisc generates RTM_DELQDISC event, in that case
>> netlink flags for user don't matter.
>
> Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
> determine it is replacement, no?
Create is RTM_NEWQDISC and NLM_F_EXCL|NLM_F_CREATE, replacement is
RTM_NEWQDISC and NLM_F_REPLACE in netlink flags.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-30 18:07 ` Roman Mashak
@ 2017-10-30 19:23 ` Cong Wang
2017-10-30 21:17 ` Roman Mashak
0 siblings, 1 reply; 9+ messages in thread
From: Cong Wang @ 2017-10-30 19:23 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
On Mon, Oct 30, 2017 at 11:07 AM, Roman Mashak <mrv@mojatatu.com> wrote:
> Cong Wang <xiyou.wangcong@gmail.com> writes:
>
>> On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>
>> Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
>> determine it is replacement, no?
>
> Create is RTM_NEWQDISC and NLM_F_EXCL|NLM_F_CREATE, replacement is
> RTM_NEWQDISC and NLM_F_REPLACE in netlink flags.
Is there any reason we can't use RTM_NEWQDISC+RTM_DELQDISC
rather than NLM_F_REPLACE to determine it is replacement?
Note, RTM_NEWQDISC+RTM_DELQDISC are put in a same
message not two.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-30 19:23 ` Cong Wang
@ 2017-10-30 21:17 ` Roman Mashak
2017-11-01 0:55 ` Cong Wang
0 siblings, 1 reply; 9+ messages in thread
From: Roman Mashak @ 2017-10-30 21:17 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Mon, Oct 30, 2017 at 11:07 AM, Roman Mashak <mrv@mojatatu.com> wrote:
>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>
>>> On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>>
>>> Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
>>> determine it is replacement, no?
>>
>> Create is RTM_NEWQDISC and NLM_F_EXCL|NLM_F_CREATE, replacement is
>> RTM_NEWQDISC and NLM_F_REPLACE in netlink flags.
>
> Is there any reason we can't use RTM_NEWQDISC+RTM_DELQDISC
> rather than NLM_F_REPLACE to determine it is replacement?
>
I'm not sure this would be valid semantics for replace operation, look at
the rfc3549:
Additional flag bits for NEW requests
NLM_F_REPLACE Replace existing matching config object with
this request.
> Note, RTM_NEWQDISC+RTM_DELQDISC are put in a same
> message not two.
Hmm, could you clarify how do you expect to put two event IDs in nlmsg_type?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-10-30 21:17 ` Roman Mashak
@ 2017-11-01 0:55 ` Cong Wang
2017-11-02 22:44 ` Roman Mashak
0 siblings, 1 reply; 9+ messages in thread
From: Cong Wang @ 2017-11-01 0:55 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
On Mon, Oct 30, 2017 at 2:17 PM, Roman Mashak <mrv@mojatatu.com> wrote:
> Cong Wang <xiyou.wangcong@gmail.com> writes:
>
>> On Mon, Oct 30, 2017 at 11:07 AM, Roman Mashak <mrv@mojatatu.com> wrote:
>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>>
>>>> On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>>>
>>>> Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
>>>> determine it is replacement, no?
>>>
>>> Create is RTM_NEWQDISC and NLM_F_EXCL|NLM_F_CREATE, replacement is
>>> RTM_NEWQDISC and NLM_F_REPLACE in netlink flags.
>>
>> Is there any reason we can't use RTM_NEWQDISC+RTM_DELQDISC
>> rather than NLM_F_REPLACE to determine it is replacement?
>>
>
> I'm not sure this would be valid semantics for replace operation, look at
> the rfc3549:
>
> Additional flag bits for NEW requests
> NLM_F_REPLACE Replace existing matching config object with
> this request.
>
I am not saying NLM_F_REPLACE is not correct, I am saying the
RTM_NEWQDISC+RTM_DELQDISC in a same message probably
exists for a reason.
>> Note, RTM_NEWQDISC+RTM_DELQDISC are put in a same
>> message not two.
>
> Hmm, could you clarify how do you expect to put two event IDs in nlmsg_type?
Looking at qdisc_notify(), it is essentially two skb_put() on a same
skb, right? So two nlmsghdr in one skb? Or I read it wrong?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification
2017-11-01 0:55 ` Cong Wang
@ 2017-11-02 22:44 ` Roman Mashak
0 siblings, 0 replies; 9+ messages in thread
From: Roman Mashak @ 2017-11-02 22:44 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Mon, Oct 30, 2017 at 2:17 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>
>>> On Mon, Oct 30, 2017 at 11:07 AM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>>>
>>>>> On Sat, Oct 28, 2017 at 8:36 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>>>> Cong Wang <xiyou.wangcong@gmail.com> writes:
>>>>>
>>>>> Hmm, I thought you use RTM_NEWQDISC+RTM_DELQDISC to
>>>>> determine it is replacement, no?
>>>>
>>>> Create is RTM_NEWQDISC and NLM_F_EXCL|NLM_F_CREATE, replacement is
>>>> RTM_NEWQDISC and NLM_F_REPLACE in netlink flags.
>>>
>>> Is there any reason we can't use RTM_NEWQDISC+RTM_DELQDISC
>>> rather than NLM_F_REPLACE to determine it is replacement?
>>>
>>
>> I'm not sure this would be valid semantics for replace operation, look at
>> the rfc3549:
>>
>> Additional flag bits for NEW requests
>> NLM_F_REPLACE Replace existing matching config object with
>> this request.
>>
>
> I am not saying NLM_F_REPLACE is not correct, I am saying the
> RTM_NEWQDISC+RTM_DELQDISC in a same message probably
> exists for a reason.
>
>
>>> Note, RTM_NEWQDISC+RTM_DELQDISC are put in a same
>>> message not two.
>>
>> Hmm, could you clarify how do you expect to put two event IDs in nlmsg_type?
>
> Looking at qdisc_notify(), it is essentially two skb_put() on a same
> skb, right? So two nlmsghdr in one skb? Or I read it wrong?
So there will be two netlink messages in a single skb, and the user
receives two events. But apparently this only happens when a new
_egress_ qdisc is being added and the default egress qdisc is deleted.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-02 22:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26 21:40 [PATCH net-next 1/1] net sched qdisc: pass netlink message flags in event notification Roman Mashak
2017-10-27 21:55 ` Cong Wang
2017-10-29 3:36 ` Roman Mashak
2017-10-30 16:23 ` Cong Wang
2017-10-30 18:07 ` Roman Mashak
2017-10-30 19:23 ` Cong Wang
2017-10-30 21:17 ` Roman Mashak
2017-11-01 0:55 ` Cong Wang
2017-11-02 22:44 ` 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).