From: Ido Schimmel <idosch@idosch•org>
To: Vladimir Nikishkin <vladimir@nikishkin•pw>,
stephen@networkplumber•org, dsahern@gmail•com
Cc: netdev@vger•kernel.org, davem@davemloft•net, edumazet@google•com,
kuba@kernel•org, pabeni@redhat•com,
eng.alaamohamedsoliman.am@gmail•com, gnault@redhat•com,
razor@blackwall•org, idosch@nvidia•com, liuhangbin@gmail•com,
eyal.birger@gmail•com, jtoppins@redhat•com
Subject: Re: [PATCH iproute2-next v5] ip-link: add support for nolocalbypass in vxlan
Date: Sun, 21 May 2023 22:23:25 +0300 [thread overview]
Message-ID: <ZGpvrV4FGjBvqVjg@shredder> (raw)
In-Reply-To: <20230521054948.22753-1-vladimir@nikishkin.pw>
On Sun, May 21, 2023 at 01:49:48PM +0800, Vladimir Nikishkin wrote:
> + if (tb[IFLA_VXLAN_LOCALBYPASS] &&
> + rta_getattr_u8(tb[IFLA_VXLAN_LOCALBYPASS])) {
^ Unaligned
> + print_bool(PRINT_ANY, "localbypass", "localbypass", true);
Missing space after "localbypass":
# ip -d link show dev vxlan0
10: vxlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/ether ce:10:63:1d:f3:a8 brd ff:ff:ff:ff:ff:ff promiscuity 0 allmulti 0 minmtu 68 maxmtu 65535
vxlan id 10 srcport 0 0 dstport 4789 ttl auto ageing 300 udpcsum localbypassnoudp6zerocsumtx [...]
Should be:
print_bool(PRINT_ANY, "localbypass", "localbypass ", true);
> + }
Parenthesis are unnecessary in this case.
I disagree with Stephen's comment about "Use presence as a boolean in
JSON". I don't like the fact that we don't have JSON output when
"false":
# ip -d -j -p link show dev vxlan0 | jq '.[]["linkinfo"]["info_data"]["localbypass"]'
true
# ip link set dev vxlan0 type vxlan nolocalbypass
# ip -d -j -p link show dev vxlan0 | jq '.[]["linkinfo"]["info_data"]["localbypass"]'
null
It's inconsistent with other iproute2 utilities such as "bridge" and
looks very much like an oversight in the initial JSON output
implementation.
IOW, I don't have a problem with the code in v4 or simply:
@@ -613,6 +622,10 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
}
}
+ if (tb[IFLA_VXLAN_LOCALBYPASS])
+ print_bool(PRINT_ANY, "localbypass", "localbypass ",
+ rta_getattr_u8(tb[IFLA_VXLAN_LOCALBYPASS]));
+
if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]);
next prev parent reply other threads:[~2023-05-21 19:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-21 5:49 [PATCH iproute2-next v5] ip-link: add support for nolocalbypass in vxlan Vladimir Nikishkin
2023-05-21 19:23 ` Ido Schimmel [this message]
2023-05-21 19:47 ` Stephen Hemminger
2023-05-22 6:15 ` Ido Schimmel
2023-05-22 15:32 ` Stephen Hemminger
2023-05-23 9:31 ` Petr Machata
2023-05-23 9:39 ` Andrea Claudi
2023-05-23 9:52 ` Vladimir Nikishkin
2023-05-23 10:32 ` Andrea Claudi
2023-05-22 6:03 ` Vladimir Nikishkin
2023-05-22 6:22 ` Ido Schimmel
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=ZGpvrV4FGjBvqVjg@shredder \
--to=idosch@idosch$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=dsahern@gmail$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=eng.alaamohamedsoliman.am@gmail$(echo .)com \
--cc=eyal.birger@gmail$(echo .)com \
--cc=gnault@redhat$(echo .)com \
--cc=idosch@nvidia$(echo .)com \
--cc=jtoppins@redhat$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=liuhangbin@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=razor@blackwall$(echo .)org \
--cc=stephen@networkplumber$(echo .)org \
--cc=vladimir@nikishkin$(echo .)pw \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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