netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	David Ahern <dsahern@kernel.org>,
	Russell Strong <russell@strong.id.au>
Subject: Re: [PATCH net-next 0/4] inet: Separate DSCP from ECN bits using new dscp_t type
Date: Fri, 17 Dec 2021 23:52:24 +0100	[thread overview]
Message-ID: <20211217225224.GA4135@pc-4.home> (raw)
In-Reply-To: <87czlvazfk.fsf@toke.dk>

On Fri, Dec 17, 2021 at 06:55:43PM +0100, Toke Høiland-Jørgensen wrote:
> >> > Note that there's no equivalent of patch 3 for IPv6 (ip route), since
> >> > the tos/dsfield option is silently ignored for IPv6 routes.
> >> 
> >> Shouldn't we just start rejecting them, like for v4?
> >
> > I had some thoughs about that, but didn't talk about them in the cover
> > letter since I felt there was already enough edge cases to discuss, and
> > this one wasn't directly related to this series (the problem is there
> > regardless of this RFC).
> >
> > So, on the one hand, we have this old policy of ignoring unknown
> > netlink attributes, so it looks consistent to also ignore unused
> > structure fields.
> >
> > On the other hand, ignoring rtm_tos leads to a different behaviour than
> > what was requested. So it certainly makes sense to at least warn the
> > user. But a hard fail may break existing programs that don't clear
> > rtm_tos by mistake.
> >
> > I'm not too sure which approach is better.
> 
> So I guess you could argue that those applications were broken in the
> first place, and so an explicit reject would only expose this? Do you
> know of any applications that actually *function* while doing what you
> describe?

I don't know of any existing application that actually does. But it's
easy to imagine a developer setting only parts of the rtmsg structure
and leaving the rest uninitialised. Exposing the problem might not help
the end user, who may have no way to modify the broken program.

Also, for people using ifupdown (/etc/network/interfaces on Debian and
derivatives), rejecting a command can cancel the configuration of an
entire device section. So a stray tos option on an ip -6 route command
would now leave the network interface entirely unconfigured.

I'm not saying these situations exist, just trying to anticipate all
possible side effects.

> One thought could be to add the rejection but be prepared to back it out
> if it does turn out (during the -rc phase) that it breaks something?

Given that it's something that'd be easy to revert, maybe we can try
this approach.

> -Toke
> 


  reply	other threads:[~2021-12-17 22:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-06 18:22 [PATCH net-next 0/4] inet: Separate DSCP from ECN bits using new dscp_t type Guillaume Nault
2021-12-06 18:22 ` [PATCH net-next 1/4] ipv6: Define dscp_t and stop taking ECN bits into account in ip6-rules Guillaume Nault
2021-12-06 18:22 ` [PATCH net-next 2/4] ipv4: Stop taking ECN bits into account in ip4-rules Guillaume Nault
2021-12-06 18:22 ` [PATCH net-next 3/4] ipv4: Reject routes specifying ECN bits in rtm_tos Guillaume Nault
2021-12-06 18:22 ` [PATCH net-next 4/4] ipv4: Use dscp_t in struct fib_alias Guillaume Nault
2021-12-06 19:57 ` [PATCH net-next 0/4] inet: Separate DSCP from ECN bits using new dscp_t type Guillaume Nault
2021-12-14  0:16 ` Toke Høiland-Jørgensen
2021-12-15 16:48   ` Guillaume Nault
2021-12-15 20:40     ` Dave Taht
2021-12-17 17:55     ` Toke Høiland-Jørgensen
2021-12-17 22:52       ` Guillaume Nault [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-02-04 13:58 Guillaume Nault
2022-02-07  6:08 ` David Ahern
2022-02-07 19:03 ` Toke Høiland-Jørgensen
2022-02-08  5:10 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211217225224.GA4135@pc-4.home \
    --to=gnault@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=russell@strong.id.au \
    --cc=toke@redhat.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).