netdev - Re: [PATCH net] tc-testing: fix geneve options match in tunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200618093131.GW102436@dhcp-12-153.nay.redhat.com>
Date:   Thu, 18 Jun 2020 17:31:31 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     netdev@...r.kernel.org,
        Pieter Jansen van Vuuren 
        <pieter.jansenvanvuuren@...ronome.com>,
        Lucas Bates <lucasb@...atatu.com>,
        Simon Horman <simon.horman@...ronome.com>,
        David Miller <davem@...emloft.net>, lucien.xin@...il.com,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net] tc-testing: fix geneve options match in tunnel_key
 unit tests

On Thu, Jun 18, 2020 at 10:53:51AM +0200, Davide Caratti wrote:
> On Thu, 2020-06-18 at 16:37 +0800, Hangbin Liu wrote:
> > tc action print "geneve_opts" instead of "geneve_opt".
> > Fix the typo, or we will unable to match correct action output.
> > 
> 
> hello Hangbin,
> 
> > Fixes: cba54f9cf4ec ("tc-testing: add geneve options in tunnel_key unit tests")
> 
> this Fixes: tag is suspicious, when a tdc test is added I would expect to
> see it passing. If I well read the code, the problem has been introduced
> in iproute2, with commit 
> 
> commit f72c3ad00f3b7869e90840d0098a83cb88224892
> Author: Xin Long <lucien.xin@...il.com>
> Date:   Mon Apr 27 18:27:48 2020 +0800
> 
>     tc: m_tunnel_key: add options support for vxlan
>     
> 
> that did:
> 
> [...]
> 
> static void tunnel_key_print_geneve_options(const char *name,
> -                                           struct rtattr *attr)
> +static void tunnel_key_print_geneve_options(struct rtattr *attr)
>  {
>         struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX + 1];
>         struct rtattr *i = RTA_DATA(attr);
>         int ii, data_len = 0, offset = 0;
>         int rem = RTA_PAYLOAD(attr);
> +       char *name = "geneve_opts";
>         char strbuf[rem * 2 + 1];
>         char data[rem * 2 + 1];
>         uint8_t data_r[rem];
> @@ -421,7 +464,7 @@ static void tunnel_key_print_geneve_options(const char *name,
>  
>         open_json_array(PRINT_JSON, name);
>         print_nl();
> -       print_string(PRINT_FP, name, "\t%s ", "geneve_opt");
> +       print_string(PRINT_FP, name, "\t%s ", name);

Ah, yes, you are right.
>  
> 
> (just speculating, because I didn't try older versions of iproute2). But if my
> hypothesis is correct, then the fix should be done in iproute2, WDYT?

If you and Stephen are both agree. I'm OK to fix it on iproute2.

Thanks
Hangbin