netdev - Re: [PATCH net] selftests: forwarding: vxlan_bridge

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] [day] [month] [year] [list]

Message-ID: <20210322011628.GC2900@Leo-laptop-t470s>
Date:   Mon, 22 Mar 2021 09:16:28 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Ido Schimmel <idosch@...dia.com>
Cc:     netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
        Petr Machata <petrm@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        Guillaume Nault <gnault@...hat.com>
Subject: Re: [PATCH net] selftests: forwarding: vxlan_bridge_1d: Fix vxlan
 ecn decapsulate value

On Sun, Mar 21, 2021 at 07:20:55PM +0200, Ido Schimmel wrote:
> On Fri, Mar 19, 2021 at 10:33:14PM +0800, Hangbin Liu wrote:
> > The ECN bit defines ECT(1) = 1, ECT(0) = 2. So inner 0x02 + outer 0x01
> > should be inner ECT(0) + outer ECT(1). Based on the description of
> > __INET_ECN_decapsulate, the final decapsulate value should be
> > ECT(1). So fix the test expect value to 0x01.
> > 
> > Before the fix:
> > TEST: VXLAN: ECN decap: 01/02->0x02                                 [FAIL]
> >         Expected to capture 10 packets, got 0.
> > 
> > After the fix:
> > TEST: VXLAN: ECN decap: 01/02->0x01                                 [ OK ]
> > 
> > Fixes: a0b61f3d8ebf ("selftests: forwarding: vxlan_bridge_1d: Add an ECN decap test")
> > Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> 
> Fixes: b723748750ec ("tunnel: Propagate ECT(1) when decapsulating as recommended by RFC6040")
> 
> The commit you cited is from 2018 whereas this one is from 2020. The
> test stopped working after the latter. The reason I didn't see it is
> because this commit only changed one caller of __INET_ECN_decapsulate().
> Another caller is mlxsw which uses the function to understand how to
> program the hardware to perform decapsulation. See commit 28e450333d4d
> ("inet: Refactor INET_ECN_decapsulate()").

Hi Ido,

Thanks for this info. I don't have mlxsw card in hand, so I never
run the driver test before.

Cheers
Hangbin

> 
> After your patch I get:
> 
> TEST: VXLAN: ECN decap: 01/02->0x01                                 [FAIL]
>         Expected to capture 10 packets, got 0.
> 
> Fixed by:
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ipip.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ipip.c
> index b8b08a6a1d10..61eb34e20fde 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ipip.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ipip.c
> @@ -341,7 +341,12 @@ static int mlxsw_sp_ipip_ecn_decap_init_one(struct mlxsw_sp *mlxsw_sp,
>         u8 new_inner_ecn;
>  
>         trap_en = __INET_ECN_decapsulate(outer_ecn, inner_ecn, &set_ce);
> -       new_inner_ecn = set_ce ? INET_ECN_CE : inner_ecn;
> +       if (set_ce)
> +               new_inner_ecn = INET_ECN_CE;
> +       else if (outer_ecn == INET_ECN_ECT_1)
> +               new_inner_ecn = INET_ECN_ECT_1;
> +       else
> +               new_inner_ecn = inner_ecn;
>  
>         mlxsw_reg_tidem_pack(tidem_pl, outer_ecn, inner_ecn, new_inner_ecn,
>                              trap_en, trap_en ? MLXSW_TRAP_ID_DECAP_ECN0 : 0);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
> index e5ec595593f4..74f2c4ce7063 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c
> @@ -913,7 +913,12 @@ static int __mlxsw_sp_nve_ecn_decap_init(struct mlxsw_sp *mlxsw_sp,
>         u8 new_inner_ecn;
>  
>         trap_en = !!__INET_ECN_decapsulate(outer_ecn, inner_ecn, &set_ce);
> -       new_inner_ecn = set_ce ? INET_ECN_CE : inner_ecn;
> +       if (set_ce)
> +               new_inner_ecn = INET_ECN_CE;
> +       else if (outer_ecn == INET_ECN_ECT_1)
> +               new_inner_ecn = INET_ECN_ECT_1;
> +       else
> +               new_inner_ecn = inner_ecn;
>  
>         mlxsw_reg_tndem_pack(tndem_pl, outer_ecn, inner_ecn, new_inner_ecn,
>                              trap_en, trap_en ? MLXSW_TRAP_ID_DECAP_ECN0 : 0);
> 
> I will prepare a patch