[PATCH] net-ipv6: changes to ->tclass (via IPV6_TCLASS) should sk_dst_reset()

From: Maciej Żenczykowski <redacted>

This is to match ipv4 behaviour, see __ip_sock_set_tos()
implementation.

Technically for ipv6 this might not be required because normally we
do not allow tclass to influence routing, yet the cli tooling does
support it:

lpk11:~# ip -6 rule add pref 5 tos 45 lookup 5
lpk11:~# ip -6 rule
5:      from all tos 0x45 lookup 5

and in general dscp/tclass based routing does make sense.

We already have cases where dscp can affect vlan priority and/or
transmit queue (especially on wifi).

So let's just make things match.  Easier to reason about and no harm.

Cc: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Maciej Żenczykowski <redacted>
---
 net/ipv6/ipv6_sockglue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 204b0b4d10c8..3a66f2394b82 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c

@@ -603,7 +603,10 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 			val &= ~INET_ECN_MASK;
 			val |= np->tclass & INET_ECN_MASK;
 		}
-		np->tclass = val;
+		if (np->tclass != val) {
+			np->tclass = val;
+			sk_dst_reset(sk);
+		}
 		retv = 0;
 		break;
 

-- 
2.34.0.rc2.393.gf8c9666880-goog

On Tue, 23 Nov 2021 14:32:08 -0800 Maciej Żenczykowski wrote:

From: Maciej Żenczykowski <redacted>

This is to match ipv4 behaviour, see __ip_sock_set_tos()
implementation.

Technically for ipv6 this might not be required because normally we
do not allow tclass to influence routing, yet the cli tooling does
support it:

lpk11:~# ip -6 rule add pref 5 tos 45 lookup 5
lpk11:~# ip -6 rule
5:      from all tos 0x45 lookup 5

and in general dscp/tclass based routing does make sense.

We already have cases where dscp can affect vlan priority and/or
transmit queue (especially on wifi).

So let's just make things match.  Easier to reason about and no harm.

Cc: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Maciej Żenczykowski <redacted>

Please send related patches as a series. There are dependencies here
which prevent the build bot from doing its job (plus it's less work 
for me since I apply by series :)).

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski [off-list ref]:

On Tue, 23 Nov 2021 14:32:08 -0800 you wrote:

From: Maciej Żenczykowski <redacted>

This is to match ipv4 behaviour, see __ip_sock_set_tos()
implementation.

Technically for ipv6 this might not be required because normally we
do not allow tclass to influence routing, yet the cli tooling does
support it:

[...]

Here is the summary with links:
  - net-ipv6: changes to ->tclass (via IPV6_TCLASS) should sk_dst_reset()
    https://git.kernel.org/netdev/net-next/c/305e95bb893c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

On Wed, Nov 24, 2021 at 7:01 PM Jakub Kicinski [off-list ref] wrote:

On Tue, 23 Nov 2021 14:32:08 -0800 Maciej Żenczykowski wrote:

quoted

From: Maciej Żenczykowski <redacted>

This is to match ipv4 behaviour, see __ip_sock_set_tos()
implementation.

Technically for ipv6 this might not be required because normally we
do not allow tclass to influence routing, yet the cli tooling does
support it:

lpk11:~# ip -6 rule add pref 5 tos 45 lookup 5
lpk11:~# ip -6 rule
5:      from all tos 0x45 lookup 5

and in general dscp/tclass based routing does make sense.

We already have cases where dscp can affect vlan priority and/or
transmit queue (especially on wifi).

So let's just make things match.  Easier to reason about and no harm.

Cc: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Maciej Żenczykowski <redacted>

Please send related patches as a series. There are dependencies here
which prevent the build bot from doing its job (plus it's less work
for me since I apply by series :)).

Ah, sorry, while the patches were stacked on each other, they're kind
of orthogonal to each other,
and neither one actually depends on the other (outside of their being
a conflict because of touching nearby code).
That's why I sent them out separately.

Additionally, I've also noticed that often the first patch out of two
won't be merged if there's disagreement on the second, even though the
first might be entirely acceptable.