From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net-next] sock: Reset dst when changing sk_mark via setsockopt Date: Wed, 7 Nov 2018 14:58:59 -0800 Message-ID: <4a6b111a-c0d2-e12c-bc42-3c723a0297bc@gmail.com> References: <20181107223327.GA21018@konacove.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit To: David Barmann , netdev@vger.kernel.org Return-path: Received: from mail-pg1-f193.google.com ([209.85.215.193]:37560 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726896AbeKHIbe (ORCPT ); Thu, 8 Nov 2018 03:31:34 -0500 Received: by mail-pg1-f193.google.com with SMTP id c10-v6so7955831pgq.4 for ; Wed, 07 Nov 2018 14:59:01 -0800 (PST) In-Reply-To: <20181107223327.GA21018@konacove.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/07/2018 02:33 PM, David Barmann wrote: > When setting the SO_MARK socket option, the dst needs to be reset so > that a new route lookup is performed. > > This fixes the case where an application wants to change routing by > setting a new sk_mark. If this is done after some packets have already > been sent, the dst is cached and has no effect. > > Signed-off-by: David Barmann > --- > net/core/sock.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 6fcc4bc07d19..187badac24a3 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -950,10 +950,14 @@ int sock_setsockopt(struct socket *sock, int level, int optname, > clear_bit(SOCK_PASSSEC, &sock->flags); > break; > case SO_MARK: > - if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) > + if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) { > ret = -EPERM; > - else > + } else { > + struct dst_entry *dst = sk_dst_get(sk); > sk->sk_mark = val; > + sk_dst_reset(sk); sk_dst_get() and dst_release() seems extra overhead ? net/ipv4/ip_sockglue.c do_ip_setsockopt() has a similar handling for IP_TOS, and it only calls sk_dst_reset() (If the new TOS is different than the current one) So I would suggest : if (!ns_capable(...)) { ret = -EPERM; } else if (val != sk->sk_mark) { sk->sk_mark = val; sk_dst_reset(sk); }