* [PATCH iproute2] vxlan: show correct ttl inherit info
@ 2018-09-27 7:28 Hangbin Liu
2018-09-27 9:27 ` Phil Sutter
0 siblings, 1 reply; 4+ messages in thread
From: Hangbin Liu @ 2018-09-27 7:28 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, David Ahern, Phil Sutter, Hangbin Liu
We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
Otherwise show the ttl number, or auto when it is 0.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
ip/iplink_vxlan.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 831f39a..7fc0e2b 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
if (strcmp(*argv, "inherit") == 0) {
- addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
+ addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
} else if (strcmp(*argv, "auto") == 0) {
addattr8(n, 1024, IFLA_VXLAN_TTL, ttl);
} else {
@@ -527,12 +527,16 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_string(PRINT_FP, NULL, "tos %s ", "inherit");
}
- if (tb[IFLA_VXLAN_TTL])
- ttl = rta_getattr_u8(tb[IFLA_VXLAN_TTL]);
- if (is_json_context() || ttl)
- print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
- else
+ if (tb[IFLA_VXLAN_TTL_INHERIT] &&
+ rta_getattr_u8(tb[IFLA_VXLAN_TTL_INHERIT])) {
print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
+ } else if (tb[IFLA_VXLAN_TTL]) {
+ ttl = rta_getattr_u8(tb[IFLA_VXLAN_TTL]);
+ if (is_json_context() || ttl)
+ print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+ else
+ print_string(PRINT_FP, NULL, "ttl %s ", "auto");
+ }
if (tb[IFLA_VXLAN_LABEL]) {
__u32 label = rta_getattr_u32(tb[IFLA_VXLAN_LABEL]);
--
2.5.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
2018-09-27 7:28 [PATCH iproute2] vxlan: show correct ttl inherit info Hangbin Liu
@ 2018-09-27 9:27 ` Phil Sutter
2018-09-27 14:07 ` Hangbin Liu
0 siblings, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2018-09-27 9:27 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, David Ahern
Hi Hangbin,
On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> Otherwise show the ttl number, or auto when it is 0.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> ip/iplink_vxlan.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> index 831f39a..7fc0e2b 100644
> --- a/ip/iplink_vxlan.c
> +++ b/ip/iplink_vxlan.c
> @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> NEXT_ARG();
> check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> if (strcmp(*argv, "inherit") == 0) {
> - addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> + addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
So for VXLAN, the attribute is just added but with a zero value. Looking
at respective kernel code, this seems fine. Now I wonder why for Geneve,
you set the value to 1 and when displaying explicitly check whether the
attribute is there *and* non-zero. OK, looks like Geneve driver always
exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
something?
Do you know why handling of the attributes in both drivers differ?
Cheers, Phil
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
2018-09-27 9:27 ` Phil Sutter
@ 2018-09-27 14:07 ` Hangbin Liu
2018-09-27 15:22 ` Phil Sutter
0 siblings, 1 reply; 4+ messages in thread
From: Hangbin Liu @ 2018-09-27 14:07 UTC (permalink / raw)
To: Phil Sutter, netdev, Stephen Hemminger, David Ahern
On Thu, Sep 27, 2018 at 11:27:45AM +0200, Phil Sutter wrote:
> Hi Hangbin,
>
> On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> > We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> > Otherwise show the ttl number, or auto when it is 0.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > ip/iplink_vxlan.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > index 831f39a..7fc0e2b 100644
> > --- a/ip/iplink_vxlan.c
> > +++ b/ip/iplink_vxlan.c
> > @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > NEXT_ARG();
> > check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> > if (strcmp(*argv, "inherit") == 0) {
> > - addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> > + addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
>
> So for VXLAN, the attribute is just added but with a zero value. Looking
> at respective kernel code, this seems fine. Now I wonder why for Geneve,
> you set the value to 1 and when displaying explicitly check whether the
> attribute is there *and* non-zero. OK, looks like Geneve driver always
> exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
> in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
> something?
Hi Phil,
The vxlan ttl inherit exportation is just fixed by
net commit 8fd78069874 ("vxlan: fill ttl inherit info") yesterday.
> Do you know why handling of the attributes in both drivers differ?
That's because I set IFLA_VXLAN_TTL_INHERIT type to NLA_FLAG. But when fix
the geneve issue, I forgot this and set IFLA_GENEVE_TTL_INHERIT type to
NLA_U8... :(
I noticed this when post the iproute2 patch. But I thought this should have
no much influence, so I just leave it as it is.
If you think we'd better have a same behaver. We can drop the geneve iproute2
patch and post a fix to net kernel tree.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
2018-09-27 14:07 ` Hangbin Liu
@ 2018-09-27 15:22 ` Phil Sutter
0 siblings, 0 replies; 4+ messages in thread
From: Phil Sutter @ 2018-09-27 15:22 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, David Ahern
Hi Hangbin,
On Thu, Sep 27, 2018 at 10:07:51PM +0800, Hangbin Liu wrote:
> On Thu, Sep 27, 2018 at 11:27:45AM +0200, Phil Sutter wrote:
> > On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> > > We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> > > Otherwise show the ttl number, or auto when it is 0.
> > >
> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > > ---
> > > ip/iplink_vxlan.c | 16 ++++++++++------
> > > 1 file changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > > index 831f39a..7fc0e2b 100644
> > > --- a/ip/iplink_vxlan.c
> > > +++ b/ip/iplink_vxlan.c
> > > @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > > NEXT_ARG();
> > > check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> > > if (strcmp(*argv, "inherit") == 0) {
> > > - addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> > > + addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
> >
> > So for VXLAN, the attribute is just added but with a zero value. Looking
> > at respective kernel code, this seems fine. Now I wonder why for Geneve,
> > you set the value to 1 and when displaying explicitly check whether the
> > attribute is there *and* non-zero. OK, looks like Geneve driver always
> > exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
> > in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
> > something?
>
> Hi Phil,
>
> The vxlan ttl inherit exportation is just fixed by
> net commit 8fd78069874 ("vxlan: fill ttl inherit info") yesterday.
Ah, thanks!
> > Do you know why handling of the attributes in both drivers differ?
>
> That's because I set IFLA_VXLAN_TTL_INHERIT type to NLA_FLAG. But when fix
> the geneve issue, I forgot this and set IFLA_GENEVE_TTL_INHERIT type to
> NLA_U8... :(
>
> I noticed this when post the iproute2 patch. But I thought this should have
> no much influence, so I just leave it as it is.
>
> If you think we'd better have a same behaver. We can drop the geneve iproute2
> patch and post a fix to net kernel tree.
I guess it doesn't make much difference, but NLA_FLAG would be more
appropriate. If you want to spend the extra effort to fix it, I guess
now's the time - later it can't be changed anymore due to compatibility
issues (I guess).
Thanks, Phil
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-27 21:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-27 7:28 [PATCH iproute2] vxlan: show correct ttl inherit info Hangbin Liu
2018-09-27 9:27 ` Phil Sutter
2018-09-27 14:07 ` Hangbin Liu
2018-09-27 15:22 ` Phil Sutter
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).