mbox series

[v2,iproute2-next,00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

Message ID 20230418113953.818831-1-vladimir.oltean@nxp.com (mailing list archive)
Headers show
Series Add tc-mqprio and tc-taprio support for preemptible traffic classes | expand

Message

Vladimir Oltean April 18, 2023, 11:39 a.m. UTC
This is the iproute2 support for the tc program to make use of the
kernel features added in commit f7d29571ab0a ("Merge branch
'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").

The state of the man pages prior to this work was a bit unsatisfactory,
so patches 03-07 contain some man page cleanup in tc-taprio(8) and
tc-mqprio(8).

I don't know exactly what's the deal with syncing the main branch
between iproute2.git and iproute2-next.git. This patch set applies on
top of today's iproute2-next.git main branch, *merged* with today's
iproute2.git main branch. If I had formatted it directly on
iproute2-next, patch 04 would have conflicted with iproute2 change
ce4068f22db4 ("man: tc-mqprio: extend prio-tc-queue mapping with
examples"). I would recommend merging the 2 trees before applying this
series to iproute2-next.

It may be desirable for patches 01-06 to go to iproute2.git, so I've
sorted those to be first, in order to make that possible.

I also dared to sync the kernel headers and provide a commit (07/10) in
the same form as David Ahern does it. The automated script was:

  #!/bin/bash

  UAPI_FOLDER=include/uapi/
  # Built with "make -j 8 headers_install O=headers"
  KERNEL_HEADERS=/opt/net-next/headers/usr/include

  for file in $(find ${UAPI_FOLDER} -type f); do
  	filename="${file##$UAPI_FOLDER}"
  	rsync -avr "$KERNEL_HEADERS/$filename" "$file"
  done

FWIW, this kernel patch depends on frame preemption being available in
iproute2:
https://lore.kernel.org/netdev/20230418111459.811553-1-vladimir.oltean@nxp.com/

Vladimir Oltean (10):
  tc/taprio: add max-sdu to the man page SYNOPSIS section
  tc/taprio: add a size table to the examples from the man page
  tc/mqprio: fix stray ] in man page synopsis
  tc/mqprio: use words in man page to express min_rate/max_rate
    dependency on bw_rlimit
  tc/mqprio: break up synopsis into multiple lines
  tc/taprio: break up help text into multiple lines
  Update kernel headers
  utils: add max() definition
  tc/mqprio: add support for preemptible traffic classes
  tc/taprio: add support for preemptible traffic classes

 include/uapi/linux/bpf.h       |  61 +++++++++++++++----
 include/uapi/linux/pkt_sched.h |  17 ++++++
 include/utils.h                |   8 +++
 man/man8/tc-mqprio.8           |  92 ++++++++++++++++++++--------
 man/man8/tc-taprio.8           |  27 +++++++--
 tc/q_mqprio.c                  |  99 ++++++++++++++++++++++++++++++
 tc/q_taprio.c                  | 108 ++++++++++++++++++++++++---------
 7 files changed, 345 insertions(+), 67 deletions(-)

Comments

David Ahern April 22, 2023, 4:14 p.m. UTC | #1
On 4/18/23 5:39 AM, Vladimir Oltean wrote:
> This is the iproute2 support for the tc program to make use of the
> kernel features added in commit f7d29571ab0a ("Merge branch
> 'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").
> 
> The state of the man pages prior to this work was a bit unsatisfactory,
> so patches 03-07 contain some man page cleanup in tc-taprio(8) and
> tc-mqprio(8).

Thanks for updating the man pages. These should go through main first; I
can sync to main after those are applied and before your set if needed.


> 
> I don't know exactly what's the deal with syncing the main branch
> between iproute2.git and iproute2-next.git. This patch set applies on
> top of today's iproute2-next.git main branch, *merged* with today's
> iproute2.git main branch. If I had formatted it directly on
> iproute2-next, patch 04 would have conflicted with iproute2 change
> ce4068f22db4 ("man: tc-mqprio: extend prio-tc-queue mapping with
> examples"). I would recommend merging the 2 trees before applying this
> series to iproute2-next.

I merge main from time to time; just did that ...

> 
> It may be desirable for patches 01-06 to go to iproute2.git, so I've
> sorted those to be first, in order to make that possible.
> 
> I also dared to sync the kernel headers and provide a commit (07/10) in
> the same form as David Ahern does it. The automated script was:
> 
>   #!/bin/bash
> 
>   UAPI_FOLDER=include/uapi/
>   # Built with "make -j 8 headers_install O=headers"
>   KERNEL_HEADERS=/opt/net-next/headers/usr/include
> 
>   for file in $(find ${UAPI_FOLDER} -type f); do
>   	filename="${file##$UAPI_FOLDER}"
>   	rsync -avr "$KERNEL_HEADERS/$filename" "$file"
>   done

... and updated headers.

Repost the patches as 2 sets with the man page fixes targeted at main
and the new preemptible work for -next.
Vladimir Oltean April 22, 2023, 4:59 p.m. UTC | #2
On Sat, Apr 22, 2023 at 10:14:01AM -0600, David Ahern wrote:
> Thanks for updating the man pages. These should go through main first; I
> can sync to main after those are applied and before your set if needed.
> 
> ... and updated headers.
> 
> Repost the patches as 2 sets with the man page fixes targeted at main
> and the new preemptible work for -next.

So the status is as follows: patches 01-06 apply cleanly to the current
iproute2/main, and after another iproute2 -> iproute2-next merge, patch
07 can be skipped and patches 08-10 also apply cleanly to the resulting
iproute2-next branch.

Unless there are changes I need to make to the contents of the patches,
could you take these from the lists, or is that a no-no?
patchwork-bot+netdevbpf@kernel.org April 24, 2023, 4:40 p.m. UTC | #3
Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Tue, 18 Apr 2023 14:39:43 +0300 you wrote:
> This is the iproute2 support for the tc program to make use of the
> kernel features added in commit f7d29571ab0a ("Merge branch
> 'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").
> 
> The state of the man pages prior to this work was a bit unsatisfactory,
> so patches 03-07 contain some man page cleanup in tc-taprio(8) and
> tc-mqprio(8).
> 
> [...]

Here is the summary with links:
  - [v2,iproute2-next,01/10] tc/taprio: add max-sdu to the man page SYNOPSIS section
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=bad08997cfec
  - [v2,iproute2-next,02/10] tc/taprio: add a size table to the examples from the man page
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=201e2f968bd2
  - [v2,iproute2-next,03/10] tc/mqprio: fix stray ] in man page synopsis
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=4f4e2481e3a5
  - [v2,iproute2-next,04/10] tc/mqprio: use words in man page to express min_rate/max_rate dependency on bw_rlimit
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=82289b7addab
  - [v2,iproute2-next,05/10] tc/mqprio: break up synopsis into multiple lines
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=8c028693cd5f
  - [v2,iproute2-next,06/10] tc/taprio: break up help text into multiple lines
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=b54a4c9fc0cb
  - [v2,iproute2-next,07/10] Update kernel headers
    (no matching commit)
  - [v2,iproute2-next,08/10] utils: add max() definition
    (no matching commit)
  - [v2,iproute2-next,09/10] tc/mqprio: add support for preemptible traffic classes
    (no matching commit)
  - [v2,iproute2-next,10/10] tc/taprio: add support for preemptible traffic classes
    (no matching commit)

You are awesome, thank you!
David Ahern April 25, 2023, 1:47 a.m. UTC | #4
On 4/22/23 10:59 AM, Vladimir Oltean wrote:
> Unless there are changes I need to make to the contents of the patches,
> could you take these from the lists, or is that a no-no?

iproute2 follows the netdev dev model with a main tree for bug fixes and
-next tree for features. In the future please separate out the patches
and send with proper targets. If a merge is needed you can state that in
the cover letter of the set for -next.
Vladimir Oltean April 25, 2023, 12:55 p.m. UTC | #5
On Mon, Apr 24, 2023 at 07:47:31PM -0600, David Ahern wrote:
> On 4/22/23 10:59 AM, Vladimir Oltean wrote:
> > Unless there are changes I need to make to the contents of the patches,
> > could you take these from the lists, or is that a no-no?
> 
> iproute2 follows the netdev dev model with a main tree for bug fixes and
> -next tree for features. In the future please separate out the patches
> and send with proper targets. If a merge is needed you can state that in
> the cover letter of the set for -next.

I know that the trees are split and it is no coincidence that my patches
were sorted in the correct order. I've been working for 10 months on
this small feature and I was impatient to get it over with, so I wanted
to eliminate one round-trip time if possible (send to "iproute2", ask
for merge, send to "iproute2-next"). I requested this honestly thinking
that there would be no difference to the end result, only less pretentious
in terms of the process. If there is any automation (I didn't see any in
Patchwork at least) or any other reason that would justify the more
pretentious process, then again, my excuses, I plead ignorance and I
will follow it more strictly next time, but I'd also like to know it :)
David Ahern April 26, 2023, 12:42 a.m. UTC | #6
On 4/25/23 6:55 AM, Vladimir Oltean wrote:
> On Mon, Apr 24, 2023 at 07:47:31PM -0600, David Ahern wrote:
>> On 4/22/23 10:59 AM, Vladimir Oltean wrote:
>>> Unless there are changes I need to make to the contents of the patches,
>>> could you take these from the lists, or is that a no-no?
>>
>> iproute2 follows the netdev dev model with a main tree for bug fixes and
>> -next tree for features. In the future please separate out the patches
>> and send with proper targets. If a merge is needed you can state that in
>> the cover letter of the set for -next.
> 
> I know that the trees are split and it is no coincidence that my patches
> were sorted in the correct order. I've been working for 10 months on
> this small feature and I was impatient to get it over with, so I wanted
> to eliminate one round-trip time if possible (send to "iproute2", ask
> for merge, send to "iproute2-next"). I requested this honestly thinking
> that there would be no difference to the end result, only less pretentious
> in terms of the process. If there is any automation (I didn't see any in
> Patchwork at least) or any other reason that would justify the more
> pretentious process, then again, my excuses, I plead ignorance and I
> will follow it more strictly next time, but I'd also like to know it :)

Maybe the word choice here is a language issue, but it is not a
'pretentious' process, it is "the" process for submitting patches to
both networking trees and iproute2 trees. You would not send a mixed
patch set to the netdev maintainers, so don't do it for iproute2.