On 23/10/2023 10:11, Jakob Meng wrote:
On 20.10.23 12:02, Kevin Traynor wrote:
On 13/10/2023 10:07, jm...@redhat.com wrote:
From: Jakob Meng <c...@jakobmeng.de>
For better usability, the function pairs get_config() and
set_config() for netdevs should be symmetric: Options which are
accepted by set_config() should be returned by get_config() and the
latter should output valid options for set_config() only.
This patch series moves key-value pairs which are no valid options
from get_config() to the get_status() callback. The documentation in
vswitchd/vswitch.xml for status columns as well as tests have been
updated accordingly.
Compared to v4, the patch has been split into a series. Change requests
from Kevin Traynor have been incorporated, e.g. n_{r,t}xq will be
reported in dpkvhostclient status and tx-steering in the dpdk status
will be "unsupported" if the hw does not support steering traffic to
additional rxq.
The netdev dpdk classes no longer share a common get_config callback,
instead both the dpdk_class and the dpdk_vhost_client_class defines
their own callbacks. For dpdk_vhost_client_class both config options
vhost-server-path and tx-retries-max are returned which were missed in
the previous patch version.
Jakob Meng (3):
netdev-dpdk: Sync and clean {get,set}_config() callbacks.
netdev-dummy: Sync and clean {get,set}_config() callbacks.
netdev-afxdp: Sync and clean {get,set}_config() callbacks.
Documentation/intro/install/afxdp.rst | 12 +--
Documentation/topics/dpdk/phy.rst | 4 +-
lib/netdev-afxdp.c | 21 +++++-
lib/netdev-afxdp.h | 1 +
lib/netdev-dpdk.c | 104 ++++++++++++++++++--------
lib/netdev-dummy.c | 19 ++++-
lib/netdev-linux-private.h | 1 +
lib/netdev-linux.c | 4 +-
tests/pmd.at | 26 +++----
tests/system-dpdk.at | 64 ++++++++++------
vswitchd/vswitch.xml | 25 ++++++-
11 files changed, 193 insertions(+), 88 deletions(-)
Hi Jakob,
The output looks good to me. Just a few minor code related comments on the code.
@previous reviewers/committers, if anyone else is intending to review before
Jakob respins for possibly the final version, please shout now!
As it is user visible change, it's probably worth to put a note in the NEWS so
users can quickly spot if they notice a change.
Best to mention the commands/output that changed ('ovs-appctl dpctl/show' and
'ovs-vsctl get Interface <interface_name> status') and say briefly that you've
aligned set_confg/get_config and updated status etc.
Suggest to not to bother mentioning specific netdevs and just do in one update,
maybe in first patch?
thanks,
Kevin.
Good idea. How about appending this to NEWS?
Post-v3.2.0
--------------------
- OVSDB:
* ...
- ovs-appctl:
* 'ovs-appctl dpctl/show' has been changed to print valid config options
only: {configured,requested}_{rx,tx}_queues and 'rx_csum_offload' have
been dropped. Now, 'n_rxq' will be used for requested rx queues. Tx
queues cannot be changed by the user, hence 'requested_tx_queues' has
been dropped. 'lsc_interrupt_mode' has been renamed to option name
'dpdk-lsc-interrupt'.
Use 'ovs-vsctl get interface *** status' to query for values which have
previously been returned by 'ovs-appctl dpctl/show'. For example, use
'ovs-vsctl get interface *** status:n_txq' to get what was previously
returned by 'configured_tx_queues'.
* 'ovs-appctl dpctl/show' will now print all available config options like
'dpdk-devargs', '{rx,tx}-flow-ctrl-{autoneg}', 'vhost-server-path' and
'tx-retries-max' if they are set.
Wdyt?
I was thinking something shorter without mentioning individual configs
would be sufficient, but I can see the benefit of listing the individual
changed configs that could be found with a grep too. Though it would be
easier to search for config names without the {}.
Another option would be to add something short in NEWS and to mention
individual configs in *.rst to say what changed in this version. e.g.
https://github.com/openvswitch/ovs/blob/master/Documentation/topics/dpdk/phy.rst?plain=1#L38
Other opinions ? How much details do we want in NEWS ?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
