From: Jiri Pirko <jiri@resnulli•us>
To: Daniel Borkmann <daniel@iogearbox•net>
Cc: netdev@vger•kernel.org, davem@davemloft•net, nogahf@mellanox•com,
jhs@mojatatu•com, xiyou.wangcong@gmail•com, mlxsw@mellanox•com,
andrew@lunn•ch, vivien.didelot@savoirfairelinux•com,
f.fainelli@gmail•com, michael.chan@broadcom•com,
ganeshgr@chelsio•com, saeedm@mellanox•com, matanb@mellanox•com,
leonro@mellanox•com, idosch@mellanox•com,
jakub.kicinski@netronome•com, simon.horman@netronome•com,
pieter.jansenvanvuuren@netronome•com, john.hurley@netronome•com,
alexander.h.duyck@intel•com, ogerlitz@mellanox•com,
john.fastabend@gmail•com, alexei.starovoitov@gmail•com
Subject: Re: [patch net-next 3/5] net: sched: introduce block mechanism to handle netif_keep_dst calls
Date: Sat, 4 Nov 2017 14:01:37 +0100 [thread overview]
Message-ID: <20171104130137.GH2024@nanopsycho> (raw)
In-Reply-To: <59FD9796.5030904@iogearbox.net>
Sat, Nov 04, 2017 at 11:33:58AM CET, daniel@iogearbox•net wrote:
>On 11/04/2017 10:55 AM, Jiri Pirko wrote:
>> Fri, Nov 03, 2017 at 09:15:54PM CET, daniel@iogearbox•net wrote:
>> > On 11/03/2017 06:19 PM, Jiri Pirko wrote:
>> > > From: Jiri Pirko <jiri@mellanox•com>
>> > >
>> > > Couple of classifiers call netif_keep_dst directly on q->dev. That is
>> > > not possible to do directly for shared blocke where multiple qdiscs are
>> > > owning the block. So introduce a infrastructure to keep track of the
>> > > block owners in list and use this list to implement block variant of
>> > > netif_keep_dst.
>> > >
>> > > Signed-off-by: Jiri Pirko <jiri@mellanox•com>
>> > [...]
>> > > +struct tcf_block_owner_item {
>> > > + struct list_head list;
>> > > + struct Qdisc *q;
>> > > + enum tcf_block_binder_type binder_type;
>> > > +};
>> > > +
>> > > +static void
>> > > +tcf_block_owner_netif_keep_dst(struct tcf_block *block,
>> > > + struct Qdisc *q,
>> > > + enum tcf_block_binder_type binder_type)
>> > > +{
>> > > + if (block->keep_dst &&
>> > > + binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
>> >
>> > Why we need to keep dst on TCF_BLOCK_BINDER_TYPE_CLSACT_EGRESS ?
>> > I presume this enum means sch_handle_egress() ? dst is dropped
>> > later ...
>>
>> This is because of the bpf check:
>> if (fp->dst_needed && !(tp->q->flags & TCQ_F_INGRESS))
>> netif_keep_dst(qdisc_dev(tp->q));
>>
>> I just maintain the same logic here.
>
>No, that's a wrong claim, really ...
>
>clsact in general hooks into the same logic as ingress, so TC_H_CLSACT
>as major needs to reuse TC_H_INGRESS, and qdiscs set up as such set
>TCQ_F_INGRESS as flags. For clsact that means both your block binder
>types for clsact here (ingress/egress).
Ah, indeed, I missed this. I will rename TCQ_F_INGRESS to TCQ_F_CLSACT
as a part of this patchset too.
>
>Please make sure that your other changes don't have similar assumption.
They don't. Thanks for the review!
next prev parent reply other threads:[~2017-11-04 13:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-03 17:19 [patch net-next 0/5] net: sched: allow qdiscs to share filter block instances Jiri Pirko
2017-11-03 17:19 ` [patch net-next 1/5] net: sched: introduce support for multiple filter chain pointers registration Jiri Pirko
2017-11-03 17:19 ` [patch net-next 2/5] net: sched: avoid usage of tp->q in tcf_classify Jiri Pirko
2017-11-03 17:19 ` [patch net-next 3/5] net: sched: introduce block mechanism to handle netif_keep_dst calls Jiri Pirko
2017-11-03 20:15 ` Daniel Borkmann
2017-11-04 9:55 ` Jiri Pirko
2017-11-04 10:33 ` Daniel Borkmann
2017-11-04 13:01 ` Jiri Pirko [this message]
2017-11-04 21:21 ` Daniel Borkmann
2017-11-03 17:19 ` [patch net-next 4/5] net: sched: remove classid and q fields from tcf_proto Jiri Pirko
2017-11-03 17:19 ` [patch net-next 5/5] net: sched: allow ingress and clsact qdiscs to share filter blocks Jiri Pirko
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=20171104130137.GH2024@nanopsycho \
--to=jiri@resnulli$(echo .)us \
--cc=alexander.h.duyck@intel$(echo .)com \
--cc=alexei.starovoitov@gmail$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=f.fainelli@gmail$(echo .)com \
--cc=ganeshgr@chelsio$(echo .)com \
--cc=idosch@mellanox$(echo .)com \
--cc=jakub.kicinski@netronome$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=john.fastabend@gmail$(echo .)com \
--cc=john.hurley@netronome$(echo .)com \
--cc=leonro@mellanox$(echo .)com \
--cc=matanb@mellanox$(echo .)com \
--cc=michael.chan@broadcom$(echo .)com \
--cc=mlxsw@mellanox$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nogahf@mellanox$(echo .)com \
--cc=ogerlitz@mellanox$(echo .)com \
--cc=pieter.jansenvanvuuren@netronome$(echo .)com \
--cc=saeedm@mellanox$(echo .)com \
--cc=simon.horman@netronome$(echo .)com \
--cc=vivien.didelot@savoirfairelinux$(echo .)com \
--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