netdev - Re: [PATCHv2 net 2/2] vxlan: fix getting tos value from DSCP field

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20200804.124756.115062893076378926.davem@davemloft.net>
Date:   Tue, 04 Aug 2020 12:47:56 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     liuhangbin@...il.com
Cc:     netdev@...r.kernel.org, gnault@...hat.com, pmachata@...il.com,
        roopa@...ulusnetworks.com, dsahern@...nel.org, akaris@...hat.com,
        stable@...r.kernel.org
Subject: Re: [PATCHv2 net 2/2] vxlan: fix getting tos value from DSCP field

From: Hangbin Liu <liuhangbin@...il.com>
Date: Tue,  4 Aug 2020 09:43:12 +0800

> In commit 71130f29979c ("vxlan: fix tos value before xmit") we strict
> the vxlan tos value before xmit. But as IP tos field has been obsoleted
> by RFC2474, and updated by RFC3168 later. We should use new DSCP field,
> or we will lost the first 3 bits value when xmit.
> 
> Fixes: 71130f29979c ("vxlan: fix tos value before xmit")
> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>

Looking at the Fixes: tag commit more closely, it doesn't make much
sense at all to me and I think the fix is that the Fixes: commit
should be reverted.

If you pass the raw TOS into ip_tunnel_ecn_encap(), then that has the
same exact effect as your patch series here.  The ECN encap routines
will clear the ECN bits before potentially incorporating the ECN value
from the inner header etc.  The clearing of the ECN bits done by your
RT_DSCP() helper is completely unnecessary, the ECN helpers do the
right thing.  So effectively the RT_DSCP() isn't changing the tos
value at all.

I also think that your commit messages are lacking, as you fail
(especially in the Fixes: commit) to show exactly where things go
wrong.  It's always good to give example code paths and show what
happens to the TOS and/or ECN values in these places, what part of
that transformation you feel is incorrect, and what exactly you
believe the correct transformation to be.

I'm not applying this series, sorry.