From: Jakub Kicinski <kuba@kernel.org>
To: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Cc: netdev@vger.kernel.org, dev@openvswitch.org, pshelar@ovn.org,
davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
ptikhomirov@virtuozzo.com, alexander.mikhalitsyn@virtuozzo.com,
avagin@google.com, brauner@kernel.org, mark.d.gray@redhat.com,
i.maximets@ovn.org, aconole@redhat.com
Subject: Re: [PATCH net-next v2 1/3] openvswitch: allow specifying ifindex of new interfaces
Date: Tue, 23 Aug 2022 12:15:27 -0700 [thread overview]
Message-ID: <20220823121527.46efd279@kernel.org> (raw)
In-Reply-To: <0989bea9-beb3-3eb1-eb55-3438f980d973@virtuozzo.com>
On Tue, 23 Aug 2022 16:50:21 +0300 Andrey Zhadchenko wrote:
> > Are you 100% sure that all user space making this call initializes
> > dp_ifindex to 0? There is no validation in the kernel today that
> > the field is not garbage as far as I can tell.
> >
> > If you are sure, please add the appropriate analysis to the commit msg.
>
> I am not sure that *all* users set dp_ifindex to zero. At least I do not
> know about them. Primary ovs userspace ovs-vswitchd certainly set to
> zero, others like WeaveNet do it too. But there can be more?
> What is the policy regarding this? I can add a new attribute, it is not
> hard.
IDK how many user space components driving OvS exist out there.
We could risk it and reuse the field, but if we get it wrong
and don't catch the regression before the final release is cut
the result gets _really_ ugly. We will have _some_ user space
out there expecting us to use ifindex and _some_ which expects
it can pass garbage, so we can neither revert not leave it...
If that makes sense.
If using / adding attribute is not a hassle that'd be my vote.
Using fixed structs like struct ovs_header is strongly discouraged
for new families exactly because of situations like this :(
next prev parent reply other threads:[~2022-08-23 20:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-19 15:30 [PATCH net-next v2 0/3] openvswitch: allow specifying ifindex of new interfaces Andrey Zhadchenko
2022-08-19 15:30 ` [PATCH net-next v2 1/3] " Andrey Zhadchenko
2022-08-22 8:06 ` Christian Brauner
2022-08-23 1:37 ` Jakub Kicinski
2022-08-23 13:50 ` Andrey Zhadchenko
2022-08-23 19:15 ` Jakub Kicinski [this message]
2022-08-19 15:30 ` [PATCH net-next v2 2/3] openvswitch: fix memory leak at failed datapath creation Andrey Zhadchenko
2022-08-22 12:53 ` Aaron Conole
2022-08-23 1:38 ` Jakub Kicinski
2022-08-19 15:30 ` [PATCH net-next v2 3/3] openvswitch: add OVS_DP_ATTR_PER_CPU_PIDS to get requests Andrey Zhadchenko
2022-08-23 1:41 ` Jakub Kicinski
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=20220823121527.46efd279@kernel.org \
--to=kuba@kernel.org \
--cc=aconole@redhat.com \
--cc=alexander.mikhalitsyn@virtuozzo.com \
--cc=andrey.zhadchenko@virtuozzo.com \
--cc=avagin@google.com \
--cc=brauner@kernel.org \
--cc=davem@davemloft.net \
--cc=dev@openvswitch.org \
--cc=edumazet@google.com \
--cc=i.maximets@ovn.org \
--cc=mark.d.gray@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pshelar@ovn.org \
--cc=ptikhomirov@virtuozzo.com \
/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).