All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Sabrina Dubroca <sd@queasysnail.net>
Cc: Donald Hunter <donald.hunter@gmail.com>,
	Antonio Quartulli <antonio@openvpn.net>,
	netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Ralf Lici <ralf@mandelbit.com>
Subject: Re: [PATCH net 2/3] ovpn: explicitly reject netlink attr PEER_LOCAL_PORT in CMD_PEER_NEW/SET
Date: Tue, 8 Jul 2025 07:47:04 -0700	[thread overview]
Message-ID: <20250708074704.5084ccb8@kernel.org> (raw)
In-Reply-To: <aGzw2RqUP-yMaVFh@krikkit>

On Tue, 8 Jul 2025 12:20:09 +0200 Sabrina Dubroca wrote:
> Ok, I see. It's a bit verbose, especially with the nest, but adding a
> reject here and there as I was suggesting wouldn't work for per-op
> policies.

Right, it's a tricky problem to solve :(
Really, the best time to address it is when family is designed.
Even folks quite familiar with netlink make the mistake of treating
nesting as a cute way of grouping related attributes.
It is really, really counter productive to use it like that, nesting
has major drawbacks.
ethtool nesting may seem "inverted", but it's a good example of nesting
used _correctly_.

> In ovpn we should also reject attributes from GET and DEL that aren't
> currently used to match the peer we want to get/delete (ie everything
> except PEER_ID), while still being able to parse all possible peer
> attributes from the kernel's reply (only for GET). So I guess we'd
> want a different variant of the nested attribute "peer" for the
> request and reply here:

Yes, that's hard to the point of probably not being worth fixing 
at the spec level? :( We could so something like:

--- a/Documentation/netlink/specs/ovpn.yaml
+++ b/Documentation/netlink/specs/ovpn.yaml
@@ -265,6 +265,11 @@ doc: Netlink protocol to control OpenVPN network devices
         type: nest
         doc: Peer specific cipher configuration
         nested-attributes: keyconf
+      -
+        name: peer-input
+        type: nest
+        nested-attributes: peer-input
+        value: 2
   -
     name: ovpn-peer-input
     subset-of: ovpn

but the codegen today will output this "fake" attribute into the uAPI
which we don't need.

In any case. I think what I suggested is slightly better than
opencoding, even if verbose :) So I set the patches to Changes
Requested..

  reply	other threads:[~2025-07-08 14:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-03 11:45 [PATCH net 0/3] pull request: ovpn for net 2025-07-03 Antonio Quartulli
2025-07-03 11:45 ` [PATCH net 1/3] ovpn: propagate socket mark to skb in UDP Antonio Quartulli
2025-07-03 14:02   ` Sabrina Dubroca
2025-07-03 11:45 ` [PATCH net 2/3] ovpn: explicitly reject netlink attr PEER_LOCAL_PORT in CMD_PEER_NEW/SET Antonio Quartulli
2025-07-03 13:07   ` Sabrina Dubroca
2025-07-07 21:48     ` Jakub Kicinski
2025-07-08 10:20       ` Sabrina Dubroca
2025-07-08 14:47         ` Jakub Kicinski [this message]
2025-07-13 20:04           ` Antonio Quartulli
2025-07-14 14:56             ` Jakub Kicinski
2025-07-15 14:36         ` Antonio Quartulli
2025-07-15 15:06           ` Jakub Kicinski
2025-07-15 15:08             ` Antonio Quartulli
2025-07-03 11:45 ` [PATCH net 3/3] ovpn: reset GSO metadata after decapsulation Antonio Quartulli

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=20250708074704.5084ccb8@kernel.org \
    --to=kuba@kernel.org \
    --cc=antonio@openvpn.net \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=ralf@mandelbit.com \
    --cc=sd@queasysnail.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.