From: Paolo Abeni <pabeni@redhat.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>, netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>
Subject: Re: [RFC PATCH 1/3] veth: implement support for set_channel ethtool op
Date: Fri, 09 Jul 2021 16:38:02 +0200 [thread overview]
Message-ID: <9c451d25fb36bc82e602bb93e384b262be743fbf.camel@redhat.com> (raw)
In-Reply-To: <1c3d691c01121e4110f23d5947b2809d5cce056b.camel@redhat.com>
On Fri, 2021-07-09 at 12:49 +0200, Paolo Abeni wrote:
> On Fri, 2021-07-09 at 12:15 +0200, Toke Høiland-Jørgensen wrote:
> > > + if (netif_running(dev))
> > > + veth_close(dev);
> > > +
> > > + priv->num_tx_queues = ch->tx_count;
> > > + priv->num_rx_queues = ch->rx_count;
> >
> > Why can't just you use netif_set_real_num_*_queues() here directly (and
> > get rid of the priv members as above)?
>
> Uhm... I haven't thought about it. Let me try ;)
Here there is a possible problem: if the latter
netif_set_real_num_*_queues() fails, we should not change the current
configuration, so we should revert the
first netif_set_real_num_*_queues() change.
Even that additional revert operation could fail. If/when that happen
set_channel() will leave the device in a different state from both the
old one and the new one, possibly with an XDP-incompatible number of
queues.
Keeping the netif_set_real_num_*_queues() calls in veth_open() avoid
the above issue: if the queue creation is problematic, the device will
stay down.
I think the additional fields are worthy, WDYT?
Cheers,
Paolo
next prev parent reply other threads:[~2021-07-09 14:38 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-09 9:39 [RFC PATCH 0/3] veth: more flexible channels number configuration Paolo Abeni
2021-07-09 9:39 ` [RFC PATCH 1/3] veth: implement support for set_channel ethtool op Paolo Abeni
2021-07-09 10:15 ` Toke Høiland-Jørgensen
2021-07-09 10:49 ` Paolo Abeni
2021-07-09 11:36 ` Toke Høiland-Jørgensen
2021-07-09 14:38 ` Paolo Abeni [this message]
2021-07-09 15:23 ` Toke Høiland-Jørgensen
2021-07-09 16:35 ` Paolo Abeni
2021-07-09 19:56 ` Jakub Kicinski
2021-07-09 19:54 ` Jakub Kicinski
2021-07-12 1:44 ` David Ahern
2021-07-12 10:45 ` Paolo Abeni
2021-07-12 15:23 ` Jakub Kicinski
2021-07-10 8:43 ` kernel test robot
2021-07-09 9:39 ` [RFC PATCH 2/3] veth: make queues nr configurable via kernel module params Paolo Abeni
2021-07-09 10:24 ` Toke Høiland-Jørgensen
2021-07-09 15:33 ` Paolo Abeni
2021-07-09 16:12 ` Toke Høiland-Jørgensen
2021-07-09 14:25 ` kernel test robot
2021-07-09 16:59 ` kernel test robot
2021-07-10 17:52 ` kernel test robot
2021-07-10 17:52 ` [RFC PATCH] veth: veth_get_num_tx_queues() can be static kernel test robot
2021-07-09 9:39 ` [RFC PATCH 3/3] selftests: net: veth: add tests for set_channel Paolo Abeni
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=9c451d25fb36bc82e602bb93e384b262be743fbf.camel@redhat.com \
--to=pabeni@redhat.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=toke@redhat.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 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.