netdev - Re: [PATCH net-next v8 15/15] p4tc: Add P4 extern interface

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 20 Nov 2023 14:00:40 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, deb.chatterjee@...el.com, anjali.singhai@...el.com, 
	namrata.limaye@...el.com, tom@...anda.io, mleitner@...hat.com, 
	Mahesh.Shirshyad@....com, tomasz.osinski@...el.com, xiyou.wangcong@...il.com, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	vladbu@...dia.com, horms@...nel.org, daniel@...earbox.net, 
	bpf@...r.kernel.org, khalidm@...dia.com, toke@...hat.com, mattyk@...dia.com
Subject: Re: [PATCH net-next v8 15/15] p4tc: Add P4 extern interface

On Mon, Nov 20, 2023 at 11:27 AM Jiri Pirko <jiri@...nulli.us> wrote:
>
> Mon, Nov 20, 2023 at 03:02:49PM CET, jhs@...atatu.com wrote:
> >On Mon, Nov 20, 2023 at 3:22 AM Jiri Pirko <jiri@...nulli.us> wrote:
> >>
> >> Fri, Nov 17, 2023 at 01:14:43PM CET, jhs@...atatu.com wrote:
> >> >On Thu, Nov 16, 2023 at 11:42 AM Jiri Pirko <jiri@...nulli.us> wrote:
> >> >>
> >> >> Thu, Nov 16, 2023 at 03:59:48PM CET, jhs@...atatu.com wrote:
> >> >>
> >> >> [...]
> >> >>
> >> >> > include/net/p4tc.h                |  161 +++
> >> >> > include/net/p4tc_ext_api.h        |  199 +++
> >> >> > include/uapi/linux/p4tc.h         |   61 +
> >> >> > include/uapi/linux/p4tc_ext.h     |   36 +
> >> >> > net/sched/p4tc/Makefile           |    2 +-
> >> >> > net/sched/p4tc/p4tc_bpf.c         |   79 +-
> >> >> > net/sched/p4tc/p4tc_ext.c         | 2204 ++++++++++++++++++++++++++++
> >> >> > net/sched/p4tc/p4tc_pipeline.c    |   34 +-
> >> >> > net/sched/p4tc/p4tc_runtime_api.c |   10 +-
> >> >> > net/sched/p4tc/p4tc_table.c       |   57 +-
> >> >> > net/sched/p4tc/p4tc_tbl_entry.c   |   25 +-
> >> >> > net/sched/p4tc/p4tc_tmpl_api.c    |    4 +
> >> >> > net/sched/p4tc/p4tc_tmpl_ext.c    | 2221 +++++++++++++++++++++++++++++
> >> >> > 13 files changed, 5083 insertions(+), 10 deletions(-)
> >> >>
> >> >> This is for this patch. Now for the whole patchset you have:
> >> >>  30 files changed, 16676 insertions(+), 39 deletions(-)
> >> >>
> >> >> I understand that you want to fit into 15 patches with all the work.
> >> >> But sorry, patches like this are unreviewable. My suggestion is to split
> >> >> the patchset into multiple ones including smaller patches and allow
> >> >> people to digest this. I don't believe that anyone can seriously stand
> >> >> to review a patch with more than 200 lines changes.
> >> >
> >> >This specific patch is not difficult to split into two. I can do that
> >> >and send out minus the first 8 trivial patches - but not familiar with
> >> >how to do "here's part 1 of the patches" and "here's patchset two".
> >>
> >> Split into multiple patchsets and send one by one. No need to have all
> >> in at once.
> >>
> >>
> >> >There's dependency between them so not clear how patchwork and
> >>
> >> What dependency. It should compile. Introduce some basic functionality
> >> first and extend it incrementally with other patchsets. The usual way.
> >>
> >
> >Sorry, still not following:
> >Lets say i split the current patchset 1 with patch 1-8 (which are
> >trivial and have been reviewed) then make the rest into patchset 2
> >with a new set 1-8. I dont see how patchset 2 compiles unless it has
> >access to code from patchset 1. Unless patchset 1 is merged i dont see
> >how this works with patchwork or reviewers. Am i missing something?
>
> Why it would not work. Describe your motivation and plans and submit
> part of the work, the rest later on. No problem.

Sorry, still not following ;->
So push the trivial patches 1-8 for merge - then push rest in small increments?

cheers,
jamal
>
> >
> >cheers,
> >jamal
> >
> >>
> >> >reviewers would deal with it. Thoughts?
> >> >
> >> >Note: The code machinery is really repeatable; for example if you look
> >> >at the tables control you will see very similar patterns to actions
> >> >etc. i.e spending time to review one will make it easy for the rest.
> >> >
> >> >cheers,
> >> >jamal
> >> >
> >> >> [...]