From: Guillaume Nault <gnault@redhat.com>
To: Ido Schimmel <idosch@nvidia.com>
Cc: dsahern@kernel.org, netdev@vger.kernel.org, davem@davemloft.net,
kuba@kernel.org, pabeni@redhat.com, edumazet@google.com
Subject: Re: [PATCH net-next 0/6] net: fib_rules: Add DSCP selector support
Date: Tue, 1 Oct 2024 22:08:12 +0200 [thread overview]
Message-ID: <ZvxWrJHbXzxpKCFK@debian> (raw)
In-Reply-To: <Zvqrb3HYM3XogzOn@shredder.mtl.com>
On Mon, Sep 30, 2024 at 04:45:19PM +0300, Ido Schimmel wrote:
> Hi Guillaume,
>
> Sorry for the delay. Was OOO / sick. Thanks for reviewing the patches.
>
> On Fri, Sep 13, 2024 at 03:08:36PM +0200, Guillaume Nault wrote:
> > On Wed, Sep 11, 2024 at 12:37:42PM +0300, Ido Schimmel wrote:
> [...]
> > > iproute2 changes can be found here [1].
> > >
> > > [1] https://github.com/idosch/iproute2/tree/submit/dscp_rfc_v1
> >
> > Any reason for always printing numbers in the json output of this
> > iproute2 RFC? Why can't json users just use the -N parameter?
>
> Because then the JSON output is always printed as a string. Example with
> the old "tos" keyword:
>
> # ip -6 rule add tos CS1 table 100
> # ip -6 -j -p rule show tos CS1
> [ {
> "priority": 32765,
> "src": "all",
> "tos": "CS1",
> "table": "100"
> } ]
> # ip -6 -j -p -N rule show tos CS1
> [ {
> "priority": 32765,
> "src": "all",
> "tos": "0x20",
> "table": "100"
> } ]
>
> Plus, JSON output should be consumed by scripts and it doesn't make
> sense to me to use symbolic names there.
I guess that's a matter of taste then. I personally wouldn't try to
imagine what the scripts expectations are, and I'd rather let them
explicitely tell what kind of output they want. I mean, I agree that
scripts would generally want to get numbers instead of symbolic names,
but I can't see why they would _always_ want that. By forcing a numeric
value, scripts have no possibility to report symbolic names, although
that could make sense if the output isn't processed further and just
displayed to the user.
But anyway, if you really prefer the numeric-only approach, I can live
with it :).
> > I haven't checked all the /etc/iproute2/rt_* aliases, but the general
> > behaviour seems to print the human readable name for both json and
> > normal outputs, unles -N is given on the command line.
>
> dcb is also always using numeric output for JSON:
>
> # dcb app add dev swp1 dscp-prio CS1:0 CS2:1
> # dcb -j -p app show dev swp1 dscp-prio
> {
> "dscp_prio": [ [ 8,0," " ],[ 16,1," " ] ]
> }
> # dcb -j -p -N app show dev swp1 dscp-prio
> {
> "dscp_prio": [ [ 8,0," " ],[ 16,1," " ] ]
> }
>
> So there is already inconsistency in iproute2. I chose the approach that
> seemed correct to me. I don't think much thought went into always
> printing strings in JSON output other than that it was easy to
> implement.
>
> David, what is your preference?
>
next prev parent reply other threads:[~2024-10-01 20:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-11 9:37 [PATCH net-next 0/6] net: fib_rules: Add DSCP selector support Ido Schimmel
2024-09-11 9:37 ` [PATCH net-next 1/6] net: fib_rules: Add DSCP selector attribute Ido Schimmel
2024-09-13 12:03 ` Guillaume Nault
2024-09-11 9:37 ` [PATCH net-next 2/6] ipv4: fib_rules: Add DSCP selector support Ido Schimmel
2024-09-13 12:10 ` Guillaume Nault
2024-09-11 9:37 ` [PATCH net-next 3/6] ipv6: " Ido Schimmel
2024-09-13 12:21 ` Guillaume Nault
2024-09-11 9:37 ` [PATCH net-next 4/6] net: fib_rules: Enable DSCP selector usage Ido Schimmel
2024-09-13 12:26 ` Guillaume Nault
2024-09-11 9:37 ` [PATCH net-next 5/6] selftests: fib_rule_tests: Add DSCP selector match tests Ido Schimmel
2024-09-13 12:52 ` Guillaume Nault
2024-09-11 9:37 ` [PATCH net-next 6/6] selftests: fib_rule_tests: Add DSCP selector connect tests Ido Schimmel
2024-09-13 12:58 ` Guillaume Nault
2024-09-13 13:08 ` [PATCH net-next 0/6] net: fib_rules: Add DSCP selector support Guillaume Nault
2024-09-30 13:45 ` Ido Schimmel
2024-09-30 18:18 ` David Ahern
2024-10-01 20:08 ` Guillaume Nault [this message]
2024-09-13 14:31 ` David Ahern
2024-09-14 4:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZvxWrJHbXzxpKCFK@debian \
--to=gnault@redhat.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=idosch@nvidia.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).