mbox series

[v2,net-next,0/5] net: switchdev: Tracepoints

Message ID 20240130201937.1897766-1-tobias@waldekranz.com (mailing list archive)
Headers show
Series net: switchdev: Tracepoints | expand

Message

Tobias Waldekranz Jan. 30, 2024, 8:19 p.m. UTC
This series starts off (1-2/5) by creating stringifiers for common
switchdev objects. This will primarily be used by the tracepoints for
decoding switchdev notifications, but drivers could also make use of
them to provide richer debug/error messages.

Then follows two refactoring commits (3-4/5), with no (intended)
functional changes:

- 3/5: Wrap all replay callbacks in br_switchdev.c in a switchdev
       function to make it easy to trace all of these.

- 4/5: Instead of using a different format for deferred items, reuse
       the existing notification structures when enqueuing. This lets
       us share a bit more code, and it unifies the data presented by
       the tracepoints.

Finally, add the tracepoints.

v1 -> v2:

- Fixup kernel-doc comment for switchdev_call_replay

I know that there are still some warnings issued by checkpatch, but
I'm not sure how to work around them, given the nature of the mapper
macros. Please advise.

Tobias Waldekranz (5):
  net: switchdev: Wrap enums in mapper macros
  net: switchdev: Add helpers to display switchdev objects as strings
  net: switchdev: Relay all replay messages through a central function
  net: switchdev: Prepare deferred notifications before enqueuing them
  net: switchdev: Add tracepoints

 include/net/switchdev.h          | 130 ++++++++----
 include/trace/events/switchdev.h |  89 ++++++++
 net/bridge/br_switchdev.c        |  10 +-
 net/switchdev/Makefile           |   2 +-
 net/switchdev/switchdev-str.c    | 278 +++++++++++++++++++++++++
 net/switchdev/switchdev.c        | 346 +++++++++++++++++--------------
 6 files changed, 650 insertions(+), 205 deletions(-)
 create mode 100644 include/trace/events/switchdev.h
 create mode 100644 net/switchdev/switchdev-str.c

Comments

Jakub Kicinski Feb. 2, 2024, 4:44 a.m. UTC | #1
On Tue, 30 Jan 2024 21:19:32 +0100 Tobias Waldekranz wrote:
> This series starts off (1-2/5) by creating stringifiers for common
> switchdev objects. This will primarily be used by the tracepoints for
> decoding switchdev notifications, but drivers could also make use of
> them to provide richer debug/error messages.
> 
> Then follows two refactoring commits (3-4/5), with no (intended)
> functional changes:
> 
> - 3/5: Wrap all replay callbacks in br_switchdev.c in a switchdev
>        function to make it easy to trace all of these.
> 
> - 4/5: Instead of using a different format for deferred items, reuse
>        the existing notification structures when enqueuing. This lets
>        us share a bit more code, and it unifies the data presented by
>        the tracepoints.
> 
> Finally, add the tracepoints.

Is there any risk with conflicting with the fixes which are getting
worked on in parallel? Sorry for not investigating myself, ENOTIME.

> v1 -> v2:
> 
> - Fixup kernel-doc comment for switchdev_call_replay
> 
> I know that there are still some warnings issued by checkpatch, but
> I'm not sure how to work around them, given the nature of the mapper
> macros. Please advise.

It's a known problem, don't worry about those.
Tobias Waldekranz Feb. 2, 2024, 7:21 a.m. UTC | #2
On tor, feb 01, 2024 at 20:44, Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 30 Jan 2024 21:19:32 +0100 Tobias Waldekranz wrote:
>> This series starts off (1-2/5) by creating stringifiers for common
>> switchdev objects. This will primarily be used by the tracepoints for
>> decoding switchdev notifications, but drivers could also make use of
>> them to provide richer debug/error messages.
>> 
>> Then follows two refactoring commits (3-4/5), with no (intended)
>> functional changes:
>> 
>> - 3/5: Wrap all replay callbacks in br_switchdev.c in a switchdev
>>        function to make it easy to trace all of these.
>> 
>> - 4/5: Instead of using a different format for deferred items, reuse
>>        the existing notification structures when enqueuing. This lets
>>        us share a bit more code, and it unifies the data presented by
>>        the tracepoints.
>> 
>> Finally, add the tracepoints.
>
> Is there any risk with conflicting with the fixes which are getting
> worked on in parallel? Sorry for not investigating myself, ENOTIME.

They will unfortunately conflict with this series, yes. My journey was:

1. There's a problem with the MDB
2. I need tracepoints to figure this out
3. Having a light down here is pretty nice, I should upstream this
4. Find/understand/fix (1)
5. (4) probably should go into net

In hindsight, I should probably have anticipated this situation and done
away with (5) before proceeding with (3).

My idea now is to get the fix accepted, wait for the next merge of net
back to net-next, then fixup this series so that it does not reintroduce
the MDB sync issue. I already have a version of the fix that applies on
top of this series, so I'll just work it in to the switchdev refactor
steps in the next version.

Is there a better way to go about this?

>> v1 -> v2:
>> 
>> - Fixup kernel-doc comment for switchdev_call_replay
>> 
>> I know that there are still some warnings issued by checkpatch, but
>> I'm not sure how to work around them, given the nature of the mapper
>> macros. Please advise.
>
> It's a known problem, don't worry about those.
Jakub Kicinski Feb. 2, 2024, 6:32 p.m. UTC | #3
On Fri, 02 Feb 2024 08:21:01 +0100 Tobias Waldekranz wrote:
> My idea now is to get the fix accepted, wait for the next merge of net
> back to net-next, then fixup this series so that it does not reintroduce
> the MDB sync issue. I already have a version of the fix that applies on
> top of this series, so I'll just work it in to the switchdev refactor
> steps in the next version.
> 
> Is there a better way to go about this?

No no, that's perfect! This set was still active in patchwork
yesterday, tho, so we could have applied it by mistake..
If you realize there's a conflict it's worth sending a comment 
to the already-posted net-next series notifying maintainers of
the situation.