DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC 0/4] ethdev: rework config restore
@ 2024-09-18  9:21 Dariusz Sosnowski
  2024-09-18  9:21 ` [RFC 1/4] " Dariusz Sosnowski
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Dariusz Sosnowski @ 2024-09-18  9:21 UTC (permalink / raw)
  To: Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko; +Cc: dev

Hi all,

We have been working on optimizing the latency of calls to rte_eth_dev_start(),
on ports spawned by mlx5 PMD. Most of the work requires changes in the implementation of
.dev_start() PMD callback, but I also wanted to start a discussion regarding
configuration restore.

rte_eth_dev_start() does a few things on top of calling .dev_start() callback:

- Before calling it:
    - eth_dev_mac_restore() - if device supports RTE_ETH_DEV_NOLIVE_MAC_ADDR;
- After calling it:
    - eth_dev_mac_restore() - if device does not support RTE_ETH_DEV_NOLIVE_MAC_ADDR;
    - restore promiscuous config
    - restore all multicast config

eth_dev_mac_restore() iterates over all known MAC addresses -
stored in rte_eth_dev_data.mac_addrs array - and calls
.mac_addr_set() and .mac_addr_add() callbacks to apply these MAC addresses.

Promiscuous config restore checks if promiscuous mode is enabled or not,
and calls .promiscuous_enable() or .promiscuous_disable() callback.

All multicast config restore checks if all multicast mode is enabled or not,
and calls .allmulticast_enable() or .allmulticast_disable() callback.

Callbacks are called directly in all of these cases, to bypass the checks
for applying the same configuration, which exist in relevant APIs.
Checks are bypassed to force drivers to reapply the configuration.

Let's consider what happens in the following sequence of API calls.

1. rte_eth_dev_configure()
2. rte_eth_tx_queue_setup()
3. rte_eth_rx_queue_setup()
4. rte_eth_promiscuous_enable()
    - Call dev->dev_ops->promiscuous_enable()
    - Stores promiscuous state in dev->data->promiscuous
5. rte_eth_allmulticast_enable()
    - Call dev->dev_ops->allmulticast_enable()
    - Stores allmulticast state in dev->data->allmulticast
6. rte_eth_dev_start()
    - Call dev->dev_ops->dev_start()
    - Call dev->dev_ops->mac_addr_set() - apply default MAC address
    - Call dev->dev_ops->promiscuous_enable()
    - Call dev->dev_ops->allmulticast_enable()

Even though all configuration is available in dev->data after step 5,
library forces reapplying this configuration in step 6.

In mlx5 PMD case all relevant callbacks require communication with the kernel driver,
to configure the device (mlx5 PMD must create/destroy new kernel flow rules and/or
change netdev config).

mlx5 PMD handles applying all configuration in .dev_start(), so the following forced callbacks
force additional communication with the kernel. The same configuration is applied multiple times.

As an optimization, mlx5 PMD could check if a given configuration was applied,
but this would duplicate the functionality of the library
(for example rte_eth_promiscuous_enable() does not call the driver
if dev->data->promiscuous is set).

Question: Since all of the configuration is available before .dev_start() callback is called,
why ethdev library does not expect .dev_start() to take this configuration into account?
In other words, why library has to reapply the configuration?

I could not find any particular reason why configuration restore exists
as part of the process (it was in the initial DPDK commit).

The patches included in this RFC, propose a mechanism which would help with managing which drivers
rely on forceful configuration restore.
Drivers could advertise if forceful configuration restore is needed through
`RTE_ETH_DEV_*_FORCE_RESTORE` device flag. If this flag is set, then the driver in question
requires ethdev to forcefully restore configuration.

This way, if we would conclude that it makes sense for .dev_start() to handle all
starting configuration aspects, we could track which drivers still rely on configuration restore.

Dariusz Sosnowski (4):
  ethdev: rework config restore
  ethdev: omit promiscuous config restore if not required
  ethdev: omit all multicast config restore if not required
  ethdev: omit MAC address restore if not required

 lib/ethdev/rte_ethdev.c | 39 ++++++++++++++++++++++++++++++++++-----
 lib/ethdev/rte_ethdev.h | 18 ++++++++++++++++++
 2 files changed, 52 insertions(+), 5 deletions(-)

--
2.39.5


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-10-11  9:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-18  9:21 [RFC 0/4] ethdev: rework config restore Dariusz Sosnowski
2024-09-18  9:21 ` [RFC 1/4] " Dariusz Sosnowski
2024-09-18  9:21 ` [RFC 2/4] ethdev: omit promiscuous config restore if not required Dariusz Sosnowski
2024-09-18  9:22 ` [RFC 3/4] ethdev: omit all multicast " Dariusz Sosnowski
2024-09-18  9:22 ` [RFC 4/4] ethdev: omit MAC address " Dariusz Sosnowski
2024-09-29 23:31 ` [RFC 0/4] ethdev: rework config restore Ferruh Yigit
2024-10-04 19:13   ` Dariusz Sosnowski
2024-10-07  9:27     ` Konstantin Ananyev
2024-10-07 22:56       ` Ferruh Yigit
2024-10-08 17:21         ` Konstantin Ananyev
2024-10-09  1:07           ` Ferruh Yigit
2024-10-09 10:54             ` Konstantin Ananyev
2024-10-09 16:18             ` Dariusz Sosnowski
2024-10-09 23:16               ` Ferruh Yigit
2024-10-10 12:08                 ` Dariusz Sosnowski
2024-10-10 12:51                   ` Ferruh Yigit
2024-10-10 16:23                     ` Dariusz Sosnowski
2024-10-10 17:08                       ` Ferruh Yigit
2024-10-10 22:58                         ` Konstantin Ananyev
2024-10-11  0:02                           ` Ferruh Yigit
2024-10-11  8:23                             ` Dariusz Sosnowski
2024-10-11  8:29                             ` Konstantin Ananyev
2024-10-11  9:37                               ` Dariusz Sosnowski

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).