netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Xin Long <lucien.xin@gmail.com>
To: David Ahern <dsahern@gmail.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	network dev <netdev@vger.kernel.org>,
	Simon Horman <simon.horman@netronome.com>
Subject: Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata
Date: Sun, 16 Feb 2020 14:38:24 +0800	[thread overview]
Message-ID: <CADvbK_fOfEC0kG8wY_xbg_Yj4t=Y1oRKxo4h5CsYxN6Keo9YBQ@mail.gmail.com> (raw)
In-Reply-To: <793b8ff4-c04a-f962-f54f-3eae87a42963@gmail.com>

On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 2/14/20 9:18 PM, Xin Long wrote:
> > On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >>
> >> On Sat, 15 Feb 2020 01:40:27 +0800
> >> Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >>> This's not gonna work. as the output will be:
> >>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> >>> instead of
> >>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> >>
> >> JSON is typeless. Lots of values are already printed in hex
> > You may mean JSON data itself is typeless.
> > But JSON objects are typed when parsing JSON data, which includes
> > string, number, array, boolean. So it matters how to define the
> > members' 'type' in JSON data.
> >
> > For example, in python's 'json' module:
> >
> > #!/usr/bin/python2
> > import json
> > json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> > json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> > parsed_json_1 = (json.loads(json_data_1))
> > parsed_json_2 = (json.loads(json_data_2))
> > print type(parsed_json_1["hwid"])
> > print type(parsed_json_2["hwid"])
> >
> > The output is:
> > <type 'unicode'>
> > <type 'int'>
> >
> > Also, '{"result": true}' is different from '{"result": "true"}' when
> > loading it in a 3rd-party lib.
> >
> > I think the JSON data coming from iproute2 is designed to be used by
> > a 3rd-party lib to parse, not just to show to users. To keep these
> > members' original type (numbers) is more appropriate, IMO.
> >
>
> Stephen: why do you think all of the numbers should be in hex?
>
> It seems like consistency with existing output should matter more.
> ip/link_gre.c for instance prints index as an int, version as an int,
> direction as a string and only hwid in hex.
>
> Xin: any reason you did not follow the output of the existingg netdev
> based solutions?
Hi David,

Option is expressed as "version:index:dir:hwid", I made all fields
in this string of hex, just like "class:type:data" in:

commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
Author: Simon Horman <simon.horman@netronome.com>
Date:   Tue Jun 26 21:39:37 2018 -0700

    net/sched: add tunnel option support to act_tunnel_key

I'm not sure if it's good to mix multiple types in this string. wdyt?

but for the JSON data, of course, these are all numbers(not string).

Thanks.

  reply	other threads:[~2020-02-16  6:47 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 10:30 [PATCHv3 iproute2-next 0/7] iproute2: fully support for geneve/vxlan/erspan options Xin Long
2020-02-14 10:30 ` [PATCHv3 iproute2-next 1/7] iproute_lwtunnel: add options support for geneve metadata Xin Long
2020-02-14 10:30   ` [PATCHv3 iproute2-next 2/7] iproute_lwtunnel: add options support for vxlan metadata Xin Long
2020-02-14 10:30     ` [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata Xin Long
2020-02-14 10:30       ` [PATCHv3 iproute2-next 4/7] tc: m_tunnel_key: add options support for vxlan Xin Long
2020-02-14 10:30         ` [PATCHv3 iproute2-next 5/7] tc: m_tunnel_key: add options support for erpsan Xin Long
2020-02-14 10:30           ` [PATCHv3 iproute2-next 6/7] tc: f_flower: add options support for vxlan Xin Long
2020-02-14 10:30             ` [PATCHv3 iproute2-next 7/7] tc: f_flower: add options support for erspan Xin Long
2020-02-14 16:13       ` [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata Stephen Hemminger
2020-02-14 17:40         ` Xin Long
2020-02-15  0:21           ` Stephen Hemminger
2020-02-15  4:18             ` Xin Long
2020-02-15 16:51               ` David Ahern
2020-02-16  6:38                 ` Xin Long [this message]
2020-02-17 19:53                   ` David Ahern
2020-02-17 21:02                     ` Stephen Hemminger
2020-02-18  4:29                       ` Xin Long
2020-04-19  8:39                         ` Xin Long
2020-04-19 22:28                           ` David Ahern
2020-04-23 11:06                             ` Xin Long
2020-04-26 18:29                             ` David Ahern
2020-04-27  5:51                               ` Xin Long
2020-04-23 15:23           ` Stephen Hemminger
2020-04-23 18:03             ` Jakub Kicinski
2020-04-27 12:38               ` David Ahern
2020-04-27 23:07                 ` Stephen Hemminger
2020-04-28  7:22             ` Xin Long

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='CADvbK_fOfEC0kG8wY_xbg_Yj4t=Y1oRKxo4h5CsYxN6Keo9YBQ@mail.gmail.com' \
    --to=lucien.xin@gmail.com \
    --cc=dsahern@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=simon.horman@netronome.com \
    --cc=stephen@networkplumber.org \
    /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).