mbox series

[net-next,0/8] net: sched: cls: add extack support

Message ID 20180116172027.22128-1-aring@mojatatu.com
Headers show
Series net: sched: cls: add extack support | expand

Message

Alexander Aring Jan. 16, 2018, 5:20 p.m. UTC
Hi,

this patch adds extack support for TC classifier subsystem. The first
patch fixes some code style issues for this patch series pointed out
by checkpatch. The other patches until the last one prepares extack
handling for the TC classifier subsystem and handle generic extack
errors.

The last patch is an example for u32 classifier to add extack support
inside the callbacks delete and change. There exists a init callback as
well, but most classifier implementation run a kalloc() once to allocate
something. Not necessary _yet_ to add extack support now.

I know there are patches around which makes changes to these files.
I will rebase my stuff on Jiri's patches if they get in before mine.

- Alex

Cc: David Ahern <dsahern@gmail.com>

Alexander Aring (8):
  net: sched: cls: fix code style issues
  net: sched: cls_api: handle generic cls errors
  net: sched: cls: add extack support for change callback
  net: sched: cls: add extack support for tcf_exts_validate
  net: sched: cls: add extack support for delete callback
  net: sched: cls: add extack support for tcf_change_indev
  net: sched: cls: add extack support for tc_setup_cb_call
  net: sched: cls_u32: add extack support

 include/net/pkt_cls.h     |  13 ++++--
 include/net/sch_generic.h |   7 +++-
 net/sched/cls_api.c       |  74 +++++++++++++++++++++++++--------
 net/sched/cls_basic.c     |  14 ++++---
 net/sched/cls_bpf.c       |  32 ++++++++------
 net/sched/cls_cgroup.c    |   9 ++--
 net/sched/cls_flow.c      |   8 ++--
 net/sched/cls_flower.c    |  31 ++++++++------
 net/sched/cls_fw.c        |  17 ++++----
 net/sched/cls_matchall.c  |  24 +++++++----
 net/sched/cls_route.c     |  12 +++---
 net/sched/cls_rsvp.h      |   7 ++--
 net/sched/cls_tcindex.c   |  14 ++++---
 net/sched/cls_u32.c       | 104 ++++++++++++++++++++++++++++++++--------------
 14 files changed, 243 insertions(+), 123 deletions(-)

Comments

Jakub Kicinski Jan. 16, 2018, 9:46 p.m. UTC | #1
On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:
> Hi,
> 
> this patch adds extack support for TC classifier subsystem. The first
> patch fixes some code style issues for this patch series pointed out
> by checkpatch. The other patches until the last one prepares extack
> handling for the TC classifier subsystem and handle generic extack
> errors.
> 
> The last patch is an example for u32 classifier to add extack support
> inside the callbacks delete and change. There exists a init callback as
> well, but most classifier implementation run a kalloc() once to allocate
> something. Not necessary _yet_ to add extack support now.
> 
> I know there are patches around which makes changes to these files.
> I will rebase my stuff on Jiri's patches if they get in before mine.

Ugh, this is going to conflict with our series too :(  (and I CCed you
on ours)

Would it be OK for you to hold off until Jiri's code gets merged and
ours comes down via bpf-next?  That shouldn't take long at all.  The
conflicts between bpf/bpf-next/net-next are really taking their toll 
on us this release cycles, I would really appreciate if we could make
some progress on this relatively simple series at least...
Jamal Hadi Salim Jan. 16, 2018, 10:12 p.m. UTC | #2
On 18-01-16 04:46 PM, Jakub Kicinski wrote:
> On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:

[..]

> Ugh, this is going to conflict with our series too :(  (and I CCed you
> on ours)
> 
> Would it be OK for you to hold off until Jiri's code gets merged and
> ours comes down via bpf-next?  That shouldn't take long at all.  The
> conflicts between bpf/bpf-next/net-next are really taking their toll
> on us this release cycles, I would really appreciate if we could make
> some progress on this relatively simple series at least...
> 

I would say precedence should be Jiri's patches, Alex's patches
and then yours:
Alex's patches fix the core (cls_api.c) area with proper extack
for the core and then he has one patch to cover a specific
use case of the u32 classifier extack. Yours is only concerned
with one use case - bpf which depend on the core (that is in Alex's
patches)

cheers,
jamal
Jakub Kicinski Jan. 16, 2018, 10:41 p.m. UTC | #3
On Tue, 16 Jan 2018 17:12:57 -0500, Jamal Hadi Salim wrote:
> On 18-01-16 04:46 PM, Jakub Kicinski wrote:
> > On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:  
> 
> [..]
> 
> > Ugh, this is going to conflict with our series too :(  (and I CCed you
> > on ours)
> > 
> > Would it be OK for you to hold off until Jiri's code gets merged and
> > ours comes down via bpf-next?  That shouldn't take long at all.  The
> > conflicts between bpf/bpf-next/net-next are really taking their toll
> > on us this release cycles, I would really appreciate if we could make
> > some progress on this relatively simple series at least...
> >   
> 
> I would say precedence should be Jiri's patches, Alex's patches
> and then yours:
> Alex's patches fix the core (cls_api.c) area with proper extack
> for the core and then he has one patch to cover a specific
> use case of the u32 classifier extack. Yours is only concerned
> with one use case - bpf which depend on the core (that is in Alex's
> patches)

Our patches are concerned with propagating the extack to drivers, 
and nfp (and netdevsim) make use of it.

I'm miffed by the fact that you jumped out with this conflicting series
*after* we posted ours, and we got shot down on white space.
Jamal Hadi Salim Jan. 16, 2018, 11:27 p.m. UTC | #4
On 18-01-16 05:41 PM, Jakub Kicinski wrote:
> On Tue, 16 Jan 2018 17:12:57 -0500, Jamal Hadi Salim wrote:
>> On 18-01-16 04:46 PM, Jakub Kicinski wrote:
>>> On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:
>>
>> [..]
>>

>> I would say precedence should be Jiri's patches, Alex's patches
>> and then yours:
>> Alex's patches fix the core (cls_api.c) area with proper extack
>> for the core and then he has one patch to cover a specific
>> use case of the u32 classifier extack. Yours is only concerned
>> with one use case - bpf which depend on the core (that is in Alex's
>> patches)
> 
> Our patches are concerned with propagating the extack to drivers,
> and nfp (and netdevsim) make use of it.
> 
> I'm miffed by the fact that you jumped out with this conflicting series
> *after* we posted ours, and we got shot down on white space.

I totally empathize with the general frustration.
The general rule is we fix the core first then add users (classifiers in
this case). Note:
Alex has a _lot_ of patches that he has been trying to send for the
last little while and this one is certainly not a new set (I actually
had reviewed this set). There are others. And the rule of "fix core
first then add users" has been imposed on him as well.

cheers,
jamal
Daniel Borkmann Jan. 17, 2018, 12:08 a.m. UTC | #5
Hey David, and others, [+Alexei]

On 01/17/2018 12:27 AM, Jamal Hadi Salim wrote:
> On 18-01-16 05:41 PM, Jakub Kicinski wrote:
>> On Tue, 16 Jan 2018 17:12:57 -0500, Jamal Hadi Salim wrote:
>>> On 18-01-16 04:46 PM, Jakub Kicinski wrote:
>>>> On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:
>>>
>>> [..]
>>>
>>> I would say precedence should be Jiri's patches, Alex's patches
>>> and then yours:
>>> Alex's patches fix the core (cls_api.c) area with proper extack
>>> for the core and then he has one patch to cover a specific
>>> use case of the u32 classifier extack. Yours is only concerned
>>> with one use case - bpf which depend on the core (that is in Alex's
>>> patches)
>>
>> Our patches are concerned with propagating the extack to drivers,
>> and nfp (and netdevsim) make use of it.
>>
>> I'm miffed by the fact that you jumped out with this conflicting series
>> *after* we posted ours, and we got shot down on white space.

So I've been looking over Quentin's series just now that sits in my
bucket and it looks fine to me, but merge with this one would probably
end up badly for David. Therefore I'm proposing the following that
should hopefully be fine and work out for Alexander and Jakub/Quentin
as a consensus:

I'm getting the current bpf-next stuff as PR out in a few minutes, so
David can pull this in and therefore net-next will also have the
dependency on nfp for Quentin's series. Then, given this one here
needs another respin anyway, I would suggest to combine the missing
patches from Alexander's series, and get it all out in a single patch
series directly for net-next w/o any interdependency hassle.

Thanks,
Daniel
Daniel Borkmann Jan. 17, 2018, 1:10 a.m. UTC | #6
On 01/17/2018 01:08 AM, Daniel Borkmann wrote:
> Hey David, and others, [+Alexei]
> 
> On 01/17/2018 12:27 AM, Jamal Hadi Salim wrote:
>> On 18-01-16 05:41 PM, Jakub Kicinski wrote:
>>> On Tue, 16 Jan 2018 17:12:57 -0500, Jamal Hadi Salim wrote:
>>>> On 18-01-16 04:46 PM, Jakub Kicinski wrote:
>>>>> On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:
>>>>
>>>> [..]
>>>>
>>>> I would say precedence should be Jiri's patches, Alex's patches
>>>> and then yours:
>>>> Alex's patches fix the core (cls_api.c) area with proper extack
>>>> for the core and then he has one patch to cover a specific
>>>> use case of the u32 classifier extack. Yours is only concerned
>>>> with one use case - bpf which depend on the core (that is in Alex's
>>>> patches)
>>>
>>> Our patches are concerned with propagating the extack to drivers,
>>> and nfp (and netdevsim) make use of it.
>>>
>>> I'm miffed by the fact that you jumped out with this conflicting series
>>> *after* we posted ours, and we got shot down on white space.
> 
> So I've been looking over Quentin's series just now that sits in my
> bucket and it looks fine to me, but merge with this one would probably
> end up badly for David. Therefore I'm proposing the following that
> should hopefully be fine and work out for Alexander and Jakub/Quentin
> as a consensus:
> 
> I'm getting the current bpf-next stuff as PR out in a few minutes, so
> David can pull this in and therefore net-next will also have the
> dependency on nfp for Quentin's series. Then, given this one here
> needs another respin anyway, I would suggest to combine the missing
> patches from Alexander's series, and get it all out in a single patch
> series directly for net-next w/o any interdependency hassle.

Ok, bpf-next PR with the nfp dependencies is now out, so all this can
make progress here. I've therefore purged Jakub's extack series from
bpf queue, so a combined series can target net-next directly then.

Thanks,
Daniel