lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAM0EoMnJszhTDFuYZHojEZtfNueHe_WDAVXgLVWNSOtoZ2KapQ@mail.gmail.com> Date: Sat, 7 Oct 2023 07:06:43 -0400 From: Jamal Hadi Salim <jhs@...atatu.com> To: Jiri Pirko <jiri@...nulli.us> Cc: Jakub Kicinski <kuba@...nel.org>, Victor Nogueira <victor@...atatu.com>, xiyou.wangcong@...il.com, davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com, mleitner@...hat.com, vladbu@...dia.com, simon.horman@...igine.com, pctammela@...atatu.com, netdev@...r.kernel.org, kernel@...atatu.com Subject: Re: [PATCH net-next v4 0/3] net/sched: Introduce tc block ports tracking and use On Sat, Oct 7, 2023 at 6:20 AM Jiri Pirko <jiri@...nulli.us> wrote: > > Sat, Oct 07, 2023 at 01:00:00AM CEST, jhs@...atatu.com wrote: > >On Fri, Oct 6, 2023 at 6:25 PM Jakub Kicinski <kuba@...nel.org> wrote: > >> > >> On Fri, 6 Oct 2023 15:06:45 -0400 Jamal Hadi Salim wrote: > >> > > I don't understand the need for configuration less here. You don't have > >> > > it for the rest of the actions. Why this is special? > >> > >> +1, FWIW > > > >We dont have any rule that says all actions MUST have parameters ;-> > >There is nothing speacial about any action that doesnt have a > >parameter. > > You are getting the configuration from the block/device the action is > attached to. Can you point me to another action doing that? We are entering a pedantic road i am afraid. If there is no existing action that has zero config then consider this one the first one. We use skb->metadata all the time as a source of information for actions, classifiers, qdiscs. If i dont need config i dont need to invent one just because, well, all other actions are using one or more config;-> Your suggestion to specify an extra config to select a block - which may be different than the one the one packet on - is a useful feature(it just adds more code) but really should be optional. i.e if you dont specify a block id configuration then we pick the metadata one. > >If we can adequately cleanup mirred, then we can put it there but > >certainly now we are adding more buttons to click on mirred. It may > >make sense to refactor the mirred code then reuse the refactored code > >in a new action. > > I don't understand why you need any new action. mirred redirect to block > instead of dev is exactly what you need. Isn't it? The actions have different meanings and lumping them together then selecting via a knob is not the right approach. There's shared code for sure. Infact the sending code was ripped from mirred so as not to touch the rest because like i said mirred has since grown a couple of horns and tails. In retrospect mirred should have been two actions with shared code - but it is too late to change now because it is very widely used. If someone like me was afraid of touching it is because there's a maintainance challenge. I consider it in the same zone as trying to restructure something in the skb. I agree mirroring to a group of ports with a simple config is a useful feature. Mirroring to a group via a tc block is simpler but adding a list of ports instead is more powerful. So this feature is useful to have in mirred - just the adding of yet one more button to say "skip this port" is my concern. Lets see how the refactoring goes first then it will be clearer - it is a lot of delicate work - but you are right lets give it some love now. cheers, jamal >
Powered by blists - more mailing lists