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?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
