From: Jamal Hadi Salim <jhs@mojatatu•com>
To: Cong Wang <xiyou.wangcong@gmail•com>
Cc: Linux Kernel Network Developers <netdev@vger•kernel.org>,
"David S. Miller" <davem@davemloft•net>
Subject: Re: [PATCH net-next v4 3/8] net_sched: mirred: remove action when the target device is gone
Date: Mon, 23 Dec 2013 17:30:36 -0500 [thread overview]
Message-ID: <52B8B98C.7050200@mojatatu.com> (raw)
In-Reply-To: <CAM_iQpWarY7kHNCtVdYqMVdADFbCUmDsAVQz1+iEx9JKsdZN5w@mail.gmail.com>
On 12/23/13 17:14, Cong Wang wrote:
> On Mon, Dec 23, 2013 at 12:41 PM, Jamal Hadi Salim <jhs@mojatatu•com> wrote:
>>
>> Now if you have such a graph:
>
> Making such an abstraction only helps misunderstanding, really...
>
So I used a graph that maps to a netdevice, qdisc, filter and actions
like you asked to - there is still a misunderstanding?
>> What you cannot do is, on your own as kernel, decide you want to delete
>> one action that is _bound_ to a filter because one of its attributes is
>> gone berserk. It doesnt matter whether such an action is mirred or foo
>> or whether D and E dont exist. You can put a big hole where the town
>> used to be and leave roads leading to the town.
>
> Again, since (non-shared) actions attached to a filter are chained by
> a doubly linked list, why not?
>
>> It will still be referenced by things preeceding it (primarily the
>> classifier which keeps an action chain intact), which is bad when the
>> next packet arrives.
>
> You know, there is a head for such linked list for the filter to refer,
> so what's the problem with deleting a node from a linked list if
> we lock it properly? In non-shared case, no filter can refer to any
> action without the head of the chain, right? So what is the problem
> of "referenced by things preeceding it" when they are linked and locked
> properly?
>
Huh? This has nothing to do with whether you are capable to remove a
node from a list. It is about you making a decision that the node should
be removed. You dont have sufficient information to make such a decision.
Remember earlier i said you are making a macroscopic decision by looking
at microscope? I just saw a white blood cell, let me chop that toe.
>> You let the user/control program which is monitoring things do that
>> and "reroute" around the problem. If you insist on putting a little part
>> of the medula oblangata in the stomach, then:
>> the only correct way to do it is delete the filter.
>
> Apparent not, different actions can attached to the same filter
> and only mirred action has a target dev.
>
Parse error.
>> And you start doing that you are making some serious policy decisions
>> in the kernel and adding lots of complexity.
>>
>
> Why removing a filter when the netdev is removed is not policy?
> Actually it is, ONLY that it is not be able to shared with current
> implementation.
>
Of course it is. Since you understand that why do you think removing
an action which is part of that policy is a good idea?
> Since you love mechanism _so much_, why not make filters shared
> by other qdisc's and stop removing them when netdev is gone in kernel?
>
> Be realistic, Jamal, user-space is hard, you can't simply let user-space
> decide everything, especially when you don't provide a simple way to do it.
> Netlink is already hard to use, even with libnl, since it enforces a cache
> layer. Sit down and spend some time to write some libnl code, compare it
> with this patch.
>
You gotta be joking. I live through this every ither day.
Frankly, I think i will have no progress if i tried to convince you the
world is round. I am dropping from this discussion.
cheers,
jamal
next prev parent reply other threads:[~2013-12-23 22:30 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-16 4:15 [PATCH net-next v4 0/8] net_sched: some cleanup and improvments Cong Wang
2013-12-16 4:15 ` [PATCH net-next v4 1/8] net_sched: remove get_stats from tc_action_ops Cong Wang
2013-12-18 14:14 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 2/8] net_sched: act: use standard struct list_head Cong Wang
2013-12-18 14:22 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 3/8] net_sched: mirred: remove action when the target device is gone Cong Wang
2013-12-18 14:31 ` Jamal Hadi Salim
2013-12-18 18:36 ` Cong Wang
2013-12-18 18:50 ` David Miller
2013-12-18 19:50 ` Jamal Hadi Salim
2013-12-18 21:42 ` Cong Wang
2013-12-22 16:15 ` Jamal Hadi Salim
2013-12-22 19:42 ` Cong Wang
2013-12-22 20:31 ` Jamal Hadi Salim
2013-12-22 21:11 ` Cong Wang
2013-12-23 12:41 ` Jamal Hadi Salim
2013-12-23 18:50 ` Cong Wang
2013-12-23 20:41 ` Jamal Hadi Salim
2013-12-23 22:14 ` Cong Wang
2013-12-23 22:30 ` Jamal Hadi Salim [this message]
2013-12-23 22:51 ` Cong Wang
2013-12-23 23:05 ` Jamal Hadi Salim
2013-12-23 23:14 ` Cong Wang
2013-12-16 4:15 ` [PATCH net-next v4 4/8] net_sched: cls: refactor out struct tcf_ext_map Cong Wang
2013-12-18 14:36 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 5/8] net_sched: init struct tcf_hashinfo at register time Cong Wang
2013-12-18 14:37 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 6/8] net_sched: convert tcf_hashinfo to hlist and use spinlock Cong Wang
2013-12-18 14:38 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 7/8] net_sched: convert tc_action_ops to use struct list_head Cong Wang
2013-12-18 14:40 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 8/8] net_sched: convert tcf_proto_ops " Cong Wang
2013-12-18 14:41 ` Jamal Hadi Salim
2013-12-16 12:45 ` [PATCH net-next v4 0/8] net_sched: some cleanup and improvments Jamal Hadi Salim
2013-12-17 21:30 ` David Miller
2013-12-18 13:21 ` Jamal Hadi Salim
2013-12-18 14:13 ` Jamal Hadi Salim
2013-12-18 18:51 ` David Miller
2013-12-18 21:40 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52B8B98C.7050200@mojatatu.com \
--to=jhs@mojatatu$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox