* [PATCH iproute2] ip: fix link type and vlan oneline output
@ 2020-01-19 1:12 Vladis Dronov
2020-01-19 1:28 ` Vladis Dronov
2020-01-20 17:34 ` Stephen Hemminger
0 siblings, 2 replies; 4+ messages in thread
From: Vladis Dronov @ 2020-01-19 1:12 UTC (permalink / raw)
To: netdev; +Cc: vdronov, David Ahern, Stephen Hemminger, George Shuklin
Move link type printing in print_linkinfo() so multiline output does not
break link options line. Add oneline support for vlan's ingress and egress
qos maps.
Before the fix:
# ip -details link show veth90.4000
5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535
vlan protocol 802.1Q id 4000 <REORDER_HDR> the option line is broken ^^^
ingress-qos-map { 1:2 }
egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
# ip -oneline -details link show veth90.4000
5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\ link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR>
ingress-qos-map { 1:2 } <<< a multiline output despite -oneline
egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
After the fix:
# ip -details link show veth90.4000
5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
vlan protocol 802.1Q id 4000 <REORDER_HDR>
ingress-qos-map { 1:2 }
egress-qos-map { 2:1 }
# ip -oneline -details link show veth90.4000
5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\ link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR> \ ingress-qos-map { 1:2 } \ egress-qos-map { 2:1 }
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206241
Reported-by: George Shuklin <george.shuklin@gmail.com>
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
ip/ipaddress.c | 6 +++---
ip/iplink_vlan.c | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 964f14df..8814c298 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1072,9 +1072,6 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
"max_mtu", "maxmtu %u ",
rta_getattr_u32(tb[IFLA_MAX_MTU]));
- if (tb[IFLA_LINKINFO])
- print_linktype(fp, tb[IFLA_LINKINFO]);
-
if (do_link && tb[IFLA_AF_SPEC])
print_af_spec(fp, tb[IFLA_AF_SPEC]);
@@ -1126,6 +1123,9 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
b1, sizeof(b1)));
}
+
+ if (tb[IFLA_LINKINFO])
+ print_linktype(fp, tb[IFLA_LINKINFO]);
}
if ((do_link || show_details) && tb[IFLA_IFALIAS]) {
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 0dfb4a8d..1e6817f5 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -183,7 +183,8 @@ static void vlan_print_map(FILE *f,
int rem;
open_json_array(PRINT_JSON, name_json);
- print_string(PRINT_FP, NULL, "\n %s { ", name_fp);
+ print_nl();
+ print_string(PRINT_FP, NULL, " %s { ", name_fp);
rem = RTA_PAYLOAD(attr);
for (i = RTA_DATA(attr); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ip: fix link type and vlan oneline output
2020-01-19 1:12 [PATCH iproute2] ip: fix link type and vlan oneline output Vladis Dronov
@ 2020-01-19 1:28 ` Vladis Dronov
2020-01-20 17:34 ` Stephen Hemminger
1 sibling, 0 replies; 4+ messages in thread
From: Vladis Dronov @ 2020-01-19 1:28 UTC (permalink / raw)
To: netdev; +Cc: vdronov, David Ahern, Stephen Hemminger, George Shuklin
hello,
i believe this fix should get to iproute2-next too and i'm not sure how
to do it. could you, please, advise/help to handle this?
-vladis
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ip: fix link type and vlan oneline output
2020-01-19 1:12 [PATCH iproute2] ip: fix link type and vlan oneline output Vladis Dronov
2020-01-19 1:28 ` Vladis Dronov
@ 2020-01-20 17:34 ` Stephen Hemminger
2020-01-21 16:59 ` Vladis Dronov
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2020-01-20 17:34 UTC (permalink / raw)
To: Vladis Dronov; +Cc: netdev, David Ahern, George Shuklin
On Sun, 19 Jan 2020 02:12:51 +0100
Vladis Dronov <vdronov@redhat.com> wrote:
> Move link type printing in print_linkinfo() so multiline output does not
> break link options line. Add oneline support for vlan's ingress and egress
> qos maps.
>
> Before the fix:
>
> # ip -details link show veth90.4000
> 5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535
> vlan protocol 802.1Q id 4000 <REORDER_HDR> the option line is broken ^^^
> ingress-qos-map { 1:2 }
> egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>
> # ip -oneline -details link show veth90.4000
> 5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\ link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR>
> ingress-qos-map { 1:2 } <<< a multiline output despite -oneline
> egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>
> After the fix:
>
> # ip -details link show veth90.4000
> 5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> vlan protocol 802.1Q id 4000 <REORDER_HDR>
> ingress-qos-map { 1:2 }
> egress-qos-map { 2:1 }
>
> # ip -oneline -details link show veth90.4000
> 5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\ link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR> \ ingress-qos-map { 1:2 } \ egress-qos-map { 2:1 }
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206241
> Reported-by: George Shuklin <george.shuklin@gmail.com>
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
Applied with changes.
The change to ipaddress.c was incorrect. You can't change the order of things in the
output.
Second, you needed to have a Fixes tag. In this case, it went back to original
vlan support.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ip: fix link type and vlan oneline output
2020-01-20 17:34 ` Stephen Hemminger
@ 2020-01-21 16:59 ` Vladis Dronov
0 siblings, 0 replies; 4+ messages in thread
From: Vladis Dronov @ 2020-01-21 16:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Ahern, George Shuklin
hello,
> The change to ipaddress.c was incorrect. You can't change the order of things
> in the output.
i still believe it is bad to break a single link layers options line with
a multi-line piece:
5: veth90.4000@veth90: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether 26:9a:05:af:db:00 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 0 maxmtu 65535
vlan protocol 802.1Q id 4000 <REORDER_HDR> the option line is broken ^^^ by \n in print_linktype()
ingress-qos-map { 1:2 }
egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
^^^ the option line continues here
print_linktype() has an unconditional \n so it always breaks a link layer
options line. this output is assumed to be human-readable, so i believe,
proper indentation is more important that an order of fields. a machine-
readable format is another one, as far as i understand.
i'm not sure how "an order of things" is supposed to be stable, if
print_linktype() outputs a variable number of variable fields (depending
on a link type) in the middle of a link layer options.
surely, if an order of things in this understanding is above all, please,
ignore my rant.
also, with the change a commit message should be different. i understand,
it is too late to change it, but still. let a proper message be at least
here, in the mail thread:
>>> begin >>>
Add oneline support for vlan's ingress and egress qos maps.
Before the fix:
# ip -oneline -details link show veth90.4000
5: veth90.4000@veth90: ... maxmtu 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR>
ingress-qos-map { 1:2 } <<< a multiline output despite -oneline
egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 ...
After the fix:
# ip -details link show veth90.4000
5: veth90.4000@veth90: ... maxmtu 65535 \ vlan protocol 802.1Q id 4000 <REORDER_HDR> \ ingress-qos-map { 1:2 } \ egress-qos-map { 2:1 } addrgenmode eui64 numtxqueues 1 ...
<<< end <<<
> Second, you needed to have a Fixes tag. In this case, it went back to
> original vlan support.
indeed, this is my mistake, thank you for correcting this in the patch
submitted.
Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer
----- Original Message -----
> From: "Stephen Hemminger" <stephen@networkplumber.org>
> To: "Vladis Dronov" <vdronov@redhat.com>
> Cc: netdev@vger.kernel.org, "David Ahern" <dsahern@gmail.com>, "George Shuklin" <george.shuklin@gmail.com>
> Sent: Monday, January 20, 2020 6:34:04 PM
> Subject: Re: [PATCH iproute2] ip: fix link type and vlan oneline output
>
> The change to ipaddress.c was incorrect. You can't change the order of things
> in the output.
>
> Second, you needed to have a Fixes tag. In this case, it went back to
> original vlan support.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-21 16:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-19 1:12 [PATCH iproute2] ip: fix link type and vlan oneline output Vladis Dronov
2020-01-19 1:28 ` Vladis Dronov
2020-01-20 17:34 ` Stephen Hemminger
2020-01-21 16:59 ` Vladis Dronov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.