Message ID | 20231030095000.720854-4-jmeng@redhat.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | netdev: Sync and clean {get, set}_config() callbacks. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
, Oct 30, 2023 at 10:50: > From: Jakob Meng <code@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 moves key-value pairs which are no valid options from > get_config() to the get_status() callback. For example, get_config() > in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues > previously. For requested rx queues the proper option name is n_rxq, > so requested_rx_queues has been renamed respectively. Tx queues > cannot be changed by the user, hence requested_tx_queues has been > dropped. Both configured_{rx,tx}_queues will be returned as > n_{r,t}xq in the get_status() callback. > > The netdev dpdk classes no longer share a common get_config() callback, > instead both the dpdk_class and the dpdk_vhost_client_class define > their own callbacks. The get_config() callback for dpdk_vhost_class has > been dropped because it does not have a set_config() callback. > > The documentation in vswitchd/vswitch.xml for status columns as well > as tests have been updated accordingly. > > Reported-at: https://bugzilla.redhat.com/1949855 > Signed-off-by: Jakob Meng <code@jakobmeng.de> Hi Jakob, this looks good. Thanks! Reviewed-by: Robin Jarry <rjarry@redhat.com>
On 30/10/2023 09:50, jmeng@redhat.com wrote: > From: Jakob Meng<code@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 moves key-value pairs which are no valid options from > get_config() to the get_status() callback. For example, get_config() > in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues > previously. For requested rx queues the proper option name is n_rxq, > so requested_rx_queues has been renamed respectively. Tx queues > cannot be changed by the user, hence requested_tx_queues has been > dropped. Both configured_{rx,tx}_queues will be returned as > n_{r,t}xq in the get_status() callback. > > The netdev dpdk classes no longer share a common get_config() callback, > instead both the dpdk_class and the dpdk_vhost_client_class define > their own callbacks. The get_config() callback for dpdk_vhost_class has > been dropped because it does not have a set_config() callback. > > The documentation in vswitchd/vswitch.xml for status columns as well > as tests have been updated accordingly. > > Reported-at:https://bugzilla.redhat.com/1949855 > Signed-off-by: Jakob Meng<code@jakobmeng.de> > --- > Documentation/topics/dpdk/phy.rst | 4 +- > NEWS | 7 ++ > lib/netdev-dpdk.c | 113 +++++++++++++++++++++--------- > tests/system-dpdk.at | 64 ++++++++++------- > vswitchd/vswitch.xml | 14 +++- > 5 files changed, 143 insertions(+), 59 deletions(-) Acked-by: Kevin Traynor <ktraynor@redhat.com>
On 10/30/23 10:50, jmeng@redhat.com wrote: > From: Jakob Meng <code@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 moves key-value pairs which are no valid options from > get_config() to the get_status() callback. For example, get_config() > in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues > previously. For requested rx queues the proper option name is n_rxq, > so requested_rx_queues has been renamed respectively. Tx queues > cannot be changed by the user, hence requested_tx_queues has been > dropped. Both configured_{rx,tx}_queues will be returned as > n_{r,t}xq in the get_status() callback. > > The netdev dpdk classes no longer share a common get_config() callback, > instead both the dpdk_class and the dpdk_vhost_client_class define > their own callbacks. The get_config() callback for dpdk_vhost_class has > been dropped because it does not have a set_config() callback. > > The documentation in vswitchd/vswitch.xml for status columns as well > as tests have been updated accordingly. > > Reported-at: https://bugzilla.redhat.com/1949855 > Signed-off-by: Jakob Meng <code@jakobmeng.de> > --- > Documentation/topics/dpdk/phy.rst | 4 +- > NEWS | 7 ++ > lib/netdev-dpdk.c | 113 +++++++++++++++++++++--------- > tests/system-dpdk.at | 64 ++++++++++------- > vswitchd/vswitch.xml | 14 +++- > 5 files changed, 143 insertions(+), 59 deletions(-) > > diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst > index f66b106c4..41cc3588a 100644 > --- a/Documentation/topics/dpdk/phy.rst > +++ b/Documentation/topics/dpdk/phy.rst > @@ -198,7 +198,7 @@ Example:: > a dedicated queue, it will be explicit:: > > $ ovs-vsctl get interface dpdk-p0 status > - {..., rx_steering=unsupported} > + {..., rx-steering=unsupported} > > More details can often be found in ``ovs-vswitchd.log``:: > > @@ -499,7 +499,7 @@ its options:: > > $ ovs-appctl dpctl/show > [...] > - port 3: dpdk-rep0 (dpdk: configured_rx_queues=1, ..., dpdk-vf-mac=00:11:22:33:44:55, ...) > + port 3: dpdk-rep0 (dpdk: ..., dpdk-vf-mac=00:11:22:33:44:55, ...) > > $ ovs-vsctl show > [...] > diff --git a/NEWS b/NEWS > index 6b45492f1..43aea97b5 100644 > --- a/NEWS > +++ b/NEWS > @@ -6,6 +6,13 @@ Post-v3.2.0 > from older version is supported but it may trigger more leader elections > during the process, and error logs complaining unrecognized fields may > be observed on old nodes. > + - ovs-appctl: > + * Output of 'dpctl/show' command no longer shows interface configuration > + status, only values of the actual configuration options, a.k.a. > + 'requested' configuration. The interface configuration status, > + a.k.a. 'configured' values, can be found in the 'status' column of > + the Interface table, i.e. with 'ovs-vsctl get interface <..> status'. > + Reported names adjusted accordingly. > > > v3.2.0 - 17 Aug 2023 > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 55700250d..29f2b280d 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1905,31 +1905,41 @@ netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args) > > ovs_mutex_lock(&dev->mutex); > > - smap_add_format(args, "requested_rx_queues", "%d", dev->user_n_rxq); > - smap_add_format(args, "configured_rx_queues", "%d", netdev->n_rxq); > - smap_add_format(args, "requested_tx_queues", "%d", dev->requested_n_txq); > - smap_add_format(args, "configured_tx_queues", "%d", netdev->n_txq); > - smap_add_format(args, "mtu", "%d", dev->mtu); > + if (dev->devargs && dev->devargs[0]) { > + smap_add_format(args, "dpdk-devargs", "%s", dev->devargs); > + } > > - if (dev->type == DPDK_DEV_ETH) { > - smap_add_format(args, "n_rxq_desc", "%d", dev->rxq_size); > - smap_add_format(args, "n_txq_desc", "%d", dev->txq_size); > - if (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) { > - smap_add(args, "rx_csum_offload", "true"); > - } else { > - smap_add(args, "rx_csum_offload", "false"); > - } > - if (dev->rx_steer_flags == DPDK_RX_STEER_LACP) { > - smap_add(args, "rx-steering", "rss+lacp"); > - } > - smap_add(args, "lsc_interrupt_mode", > - dev->lsc_interrupt_mode ? "true" : "false"); > + smap_add_format(args, "n_rxq", "%d", dev->user_n_rxq); > > - if (dpdk_port_is_representor(dev)) { > - smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT, > - ETH_ADDR_ARGS(dev->requested_hwaddr)); > - } > + if (dev->fc_conf.mode == RTE_ETH_FC_TX_PAUSE || > + dev->fc_conf.mode == RTE_ETH_FC_FULL) { > + smap_add(args, "rx-flow-ctrl", "true"); > } > + > + if (dev->fc_conf.mode == RTE_ETH_FC_RX_PAUSE || > + dev->fc_conf.mode == RTE_ETH_FC_FULL) { > + smap_add(args, "tx-flow-ctrl", "true"); > + } > + > + if (dev->fc_conf.autoneg) { > + smap_add(args, "flow-ctrl-autoneg", "true"); > + } > + > + smap_add_format(args, "n_rxq_desc", "%d", dev->rxq_size); > + smap_add_format(args, "n_txq_desc", "%d", dev->txq_size); > + > + if (dev->rx_steer_flags == DPDK_RX_STEER_LACP) { > + smap_add(args, "rx-steering", "rss+lacp"); > + } > + > + smap_add(args, "dpdk-lsc-interrupt", > + dev->lsc_interrupt_mode ? "true" : "false"); > + > + if (dpdk_port_is_representor(dev)) { > + smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT, > + ETH_ADDR_ARGS(dev->requested_hwaddr)); > + } > + > ovs_mutex_unlock(&dev->mutex); > > return 0; > @@ -2324,6 +2334,29 @@ out: > return err; > } > > +static int > +netdev_dpdk_vhost_client_get_config(const struct netdev *netdev, > + struct smap *args) > +{ > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + int tx_retries_max; > + > + ovs_mutex_lock(&dev->mutex); > + > + if (dev->vhost_id) { > + smap_add(args, "vhost-server-path", dev->vhost_id); > + } > + > + atomic_read_relaxed(&dev->vhost_tx_retries_max, &tx_retries_max); > + if (tx_retries_max != VHOST_ENQ_RETRY_DEF) { > + smap_add_format(args, "tx-retries-max", "%d", tx_retries_max); > + } > + > + ovs_mutex_unlock(&dev->mutex); > + > + return 0; > +} > + > static int > netdev_dpdk_vhost_client_set_config(struct netdev *netdev, > const struct smap *args, > @@ -4091,6 +4124,9 @@ netdev_dpdk_vhost_user_get_status(const struct netdev *netdev, > smap_add_format(args, "userspace-tso", "disabled"); > } > > + smap_add_format(args, "n_rxq", "%d", netdev->n_rxq); > + smap_add_format(args, "n_txq", "%d", netdev->n_txq); > + > ovs_mutex_unlock(&dev->mutex); > return 0; > } > @@ -4161,6 +4197,13 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args) > smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs); > smap_add_format(args, "max_vmdq_pools", "%u", dev_info.max_vmdq_pools); > > + smap_add_format(args, "n_rxq", "%d", netdev->n_rxq); > + smap_add_format(args, "n_txq", "%d", netdev->n_txq); > + > + smap_add(args, "rx_csum_offload", > + dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD > + ? "true" : "false"); > + > /* Querying the DPDK library for iftype may be done in future, pending > * support; cf. RFC 3635 Section 3.2.4. */ > enum { IF_TYPE_ETHERNETCSMACD = 6 }; > @@ -4186,16 +4229,21 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args) > ETH_ADDR_ARGS(dev->hwaddr)); > } > > - if (rx_steer_flags) { > - if (!rx_steer_flows_num) { > - smap_add(args, "rx_steering", "unsupported"); > + if (rx_steer_flags && !rx_steer_flows_num) { > + smap_add(args, "rx-steering", "unsupported"); > + } else if (rx_steer_flags == DPDK_RX_STEER_LACP) { > + smap_add(args, "rx-steering", "rss+lacp"); > + } else { > + ovs_assert(!rx_steer_flags); > + smap_add(args, "rx-steering", "rss"); > + } > + > + if (rx_steer_flags && rx_steer_flows_num) { > + smap_add_format(args, "rx_steering_queue", "%d", n_rxq - 1); > + if (n_rxq > 2) { > + smap_add_format(args, "rss_queues", "0-%d", n_rxq - 2); > } else { > - smap_add_format(args, "rx_steering_queue", "%d", n_rxq - 1); > - if (n_rxq > 2) { > - smap_add_format(args, "rss_queues", "0-%d", n_rxq - 2); > - } else { > - smap_add(args, "rss_queues", "0"); > - } > + smap_add(args, "rss_queues", "0"); > } > } > > @@ -6415,7 +6463,6 @@ parse_vhost_config(const struct smap *ovs_other_config) > .is_pmd = true, \ > .alloc = netdev_dpdk_alloc, \ > .dealloc = netdev_dpdk_dealloc, \ > - .get_config = netdev_dpdk_get_config, \ > .get_numa_id = netdev_dpdk_get_numa_id, \ > .set_etheraddr = netdev_dpdk_set_etheraddr, \ > .get_etheraddr = netdev_dpdk_get_etheraddr, \ > @@ -6459,6 +6506,7 @@ static const struct netdev_class dpdk_class = { > .type = "dpdk", > NETDEV_DPDK_CLASS_BASE, > .construct = netdev_dpdk_construct, > + .get_config = netdev_dpdk_get_config, > .set_config = netdev_dpdk_set_config, > .send = netdev_dpdk_eth_send, > }; > @@ -6485,6 +6533,7 @@ static const struct netdev_class dpdk_vhost_client_class = { > .init = netdev_dpdk_vhost_class_init, > .construct = netdev_dpdk_vhost_client_construct, > .destruct = netdev_dpdk_vhost_destruct, > + .get_config = netdev_dpdk_vhost_client_get_config, > .set_config = netdev_dpdk_vhost_client_set_config, > .send = netdev_dpdk_vhost_send, > .get_carrier = netdev_dpdk_vhost_get_carrier, > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at > index 0f58e8574..fd42aed0b 100644 > --- a/tests/system-dpdk.at > +++ b/tests/system-dpdk.at > @@ -588,8 +588,9 @@ AT_CHECK([ovs-vsctl show], [], [stdout]) > sleep 2 > > dnl Check default MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=1500' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +1500 > +]) > > dnl Increase MTU value and check in the datapath > AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9000]) > @@ -600,8 +601,9 @@ AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch > dnl Fail if error is encountered during MTU setup > AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout]) > > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +9000 > +]) > > > dnl Clean up > @@ -636,14 +638,16 @@ dnl Fail if error is encountered during MTU setup > AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +9000 > +]) > > dnl Decrease MTU value and check in the datapath > AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=2000]) > > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=2000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +2000 > +]) > > > dnl Clean up > @@ -686,16 +690,19 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ > --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > > OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) > +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) > > dnl Check default MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=1500' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +1500 > +]) > > dnl Increase MTU value and check in the datapath > AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9000]) > > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +9000 > +]) > > dnl Clean up the testpmd now > pkill -f -x -9 'tail -f /dev/null' > @@ -743,16 +750,19 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ > --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > > OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) > +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +9000 > +]) > > dnl Decrease MTU value and check in the datapath > AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=2000]) > > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=2000' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +2000 > +]) > > dnl Clean up the testpmd now > pkill -f -x -9 'tail -f /dev/null' > @@ -789,8 +799,9 @@ dnl Fail if error is encountered during MTU setup > AT_FAIL_IF([grep "Interface phy0 MTU (9702) setup error" ovs-vswitchd.log], [], [stdout]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9702' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +9702 > +]) > > dnl Set MTU value above upper bound and check for error > AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9711]) > @@ -830,8 +841,9 @@ dnl Fail if error is encountered during MTU setup > AT_FAIL_IF([grep "Interface phy0 MTU (68) setup error" ovs-vswitchd.log], [], [stdout]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=68' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl > +68 > +]) > > dnl Set MTU value below lower bound and check for error > AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=67]) > @@ -877,10 +889,12 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ > --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > > OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) > +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=9702' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +9702 > +]) > > dnl Set MTU value above upper bound and check for error > AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9711]) > @@ -934,10 +948,12 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ > --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > > OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) > +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) > > dnl Check MTU value in the datapath > -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) > -AT_CHECK([grep -E 'mtu=68' stdout], [], [stdout]) > +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > +68 > +]) > > dnl Set MTU value below lower bound and check for error > AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=67]) > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index d84260d75..87674f088 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -3789,6 +3789,18 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > Maximum number of VMDq pools. > </column> > > + <column name="status" key="n_rxq"> > + Number of rx queues. > + </column> > + > + <column name="status" key="n_txq"> > + Number of tx queues. Nit: Maybe we can be a little more consistent with rx/Rx/RX/tx/Tx/TX ? The current docs are kind of all over the place, but maybe we could use the smae style at least within a single patch? :) 'Rx/Tx' might be a style of choice, e.g. DPDK requires that format for a git log. > + </column> > + > + <column name="status" key="rx_csum_offload"> > + Whether Rx Checksum offload is enabled or not. > + </column> > + > <column name="status" key="if_type"> > Interface type ID according to IANA ifTYPE MIB definitions. > </column> > @@ -3807,7 +3819,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > VF representors. > </column> > > - <column name="status" key="rx_steering"> > + <column name="status" key="rx-steering"> > Hardware Rx queue steering policy in use. > </column> >
diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst index f66b106c4..41cc3588a 100644 --- a/Documentation/topics/dpdk/phy.rst +++ b/Documentation/topics/dpdk/phy.rst @@ -198,7 +198,7 @@ Example:: a dedicated queue, it will be explicit:: $ ovs-vsctl get interface dpdk-p0 status - {..., rx_steering=unsupported} + {..., rx-steering=unsupported} More details can often be found in ``ovs-vswitchd.log``:: @@ -499,7 +499,7 @@ its options:: $ ovs-appctl dpctl/show [...] - port 3: dpdk-rep0 (dpdk: configured_rx_queues=1, ..., dpdk-vf-mac=00:11:22:33:44:55, ...) + port 3: dpdk-rep0 (dpdk: ..., dpdk-vf-mac=00:11:22:33:44:55, ...) $ ovs-vsctl show [...] diff --git a/NEWS b/NEWS index 6b45492f1..43aea97b5 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,13 @@ Post-v3.2.0 from older version is supported but it may trigger more leader elections during the process, and error logs complaining unrecognized fields may be observed on old nodes. + - ovs-appctl: + * Output of 'dpctl/show' command no longer shows interface configuration + status, only values of the actual configuration options, a.k.a. + 'requested' configuration. The interface configuration status, + a.k.a. 'configured' values, can be found in the 'status' column of + the Interface table, i.e. with 'ovs-vsctl get interface <..> status'. + Reported names adjusted accordingly. v3.2.0 - 17 Aug 2023 diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 55700250d..29f2b280d 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1905,31 +1905,41 @@ netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args) ovs_mutex_lock(&dev->mutex); - smap_add_format(args, "requested_rx_queues", "%d", dev->user_n_rxq); - smap_add_format(args, "configured_rx_queues", "%d", netdev->n_rxq); - smap_add_format(args, "requested_tx_queues", "%d", dev->requested_n_txq); - smap_add_format(args, "configured_tx_queues", "%d", netdev->n_txq); - smap_add_format(args, "mtu", "%d", dev->mtu); + if (dev->devargs && dev->devargs[0]) { + smap_add_format(args, "dpdk-devargs", "%s", dev->devargs); + } - if (dev->type == DPDK_DEV_ETH) { - smap_add_format(args, "n_rxq_desc", "%d", dev->rxq_size); - smap_add_format(args, "n_txq_desc", "%d", dev->txq_size); - if (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) { - smap_add(args, "rx_csum_offload", "true"); - } else { - smap_add(args, "rx_csum_offload", "false"); - } - if (dev->rx_steer_flags == DPDK_RX_STEER_LACP) { - smap_add(args, "rx-steering", "rss+lacp"); - } - smap_add(args, "lsc_interrupt_mode", - dev->lsc_interrupt_mode ? "true" : "false"); + smap_add_format(args, "n_rxq", "%d", dev->user_n_rxq); - if (dpdk_port_is_representor(dev)) { - smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT, - ETH_ADDR_ARGS(dev->requested_hwaddr)); - } + if (dev->fc_conf.mode == RTE_ETH_FC_TX_PAUSE || + dev->fc_conf.mode == RTE_ETH_FC_FULL) { + smap_add(args, "rx-flow-ctrl", "true"); } + + if (dev->fc_conf.mode == RTE_ETH_FC_RX_PAUSE || + dev->fc_conf.mode == RTE_ETH_FC_FULL) { + smap_add(args, "tx-flow-ctrl", "true"); + } + + if (dev->fc_conf.autoneg) { + smap_add(args, "flow-ctrl-autoneg", "true"); + } + + smap_add_format(args, "n_rxq_desc", "%d", dev->rxq_size); + smap_add_format(args, "n_txq_desc", "%d", dev->txq_size); + + if (dev->rx_steer_flags == DPDK_RX_STEER_LACP) { + smap_add(args, "rx-steering", "rss+lacp"); + } + + smap_add(args, "dpdk-lsc-interrupt", + dev->lsc_interrupt_mode ? "true" : "false"); + + if (dpdk_port_is_representor(dev)) { + smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT, + ETH_ADDR_ARGS(dev->requested_hwaddr)); + } + ovs_mutex_unlock(&dev->mutex); return 0; @@ -2324,6 +2334,29 @@ out: return err; } +static int +netdev_dpdk_vhost_client_get_config(const struct netdev *netdev, + struct smap *args) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + int tx_retries_max; + + ovs_mutex_lock(&dev->mutex); + + if (dev->vhost_id) { + smap_add(args, "vhost-server-path", dev->vhost_id); + } + + atomic_read_relaxed(&dev->vhost_tx_retries_max, &tx_retries_max); + if (tx_retries_max != VHOST_ENQ_RETRY_DEF) { + smap_add_format(args, "tx-retries-max", "%d", tx_retries_max); + } + + ovs_mutex_unlock(&dev->mutex); + + return 0; +} + static int netdev_dpdk_vhost_client_set_config(struct netdev *netdev, const struct smap *args, @@ -4091,6 +4124,9 @@ netdev_dpdk_vhost_user_get_status(const struct netdev *netdev, smap_add_format(args, "userspace-tso", "disabled"); } + smap_add_format(args, "n_rxq", "%d", netdev->n_rxq); + smap_add_format(args, "n_txq", "%d", netdev->n_txq); + ovs_mutex_unlock(&dev->mutex); return 0; } @@ -4161,6 +4197,13 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args) smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs); smap_add_format(args, "max_vmdq_pools", "%u", dev_info.max_vmdq_pools); + smap_add_format(args, "n_rxq", "%d", netdev->n_rxq); + smap_add_format(args, "n_txq", "%d", netdev->n_txq); + + smap_add(args, "rx_csum_offload", + dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD + ? "true" : "false"); + /* Querying the DPDK library for iftype may be done in future, pending * support; cf. RFC 3635 Section 3.2.4. */ enum { IF_TYPE_ETHERNETCSMACD = 6 }; @@ -4186,16 +4229,21 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args) ETH_ADDR_ARGS(dev->hwaddr)); } - if (rx_steer_flags) { - if (!rx_steer_flows_num) { - smap_add(args, "rx_steering", "unsupported"); + if (rx_steer_flags && !rx_steer_flows_num) { + smap_add(args, "rx-steering", "unsupported"); + } else if (rx_steer_flags == DPDK_RX_STEER_LACP) { + smap_add(args, "rx-steering", "rss+lacp"); + } else { + ovs_assert(!rx_steer_flags); + smap_add(args, "rx-steering", "rss"); + } + + if (rx_steer_flags && rx_steer_flows_num) { + smap_add_format(args, "rx_steering_queue", "%d", n_rxq - 1); + if (n_rxq > 2) { + smap_add_format(args, "rss_queues", "0-%d", n_rxq - 2); } else { - smap_add_format(args, "rx_steering_queue", "%d", n_rxq - 1); - if (n_rxq > 2) { - smap_add_format(args, "rss_queues", "0-%d", n_rxq - 2); - } else { - smap_add(args, "rss_queues", "0"); - } + smap_add(args, "rss_queues", "0"); } } @@ -6415,7 +6463,6 @@ parse_vhost_config(const struct smap *ovs_other_config) .is_pmd = true, \ .alloc = netdev_dpdk_alloc, \ .dealloc = netdev_dpdk_dealloc, \ - .get_config = netdev_dpdk_get_config, \ .get_numa_id = netdev_dpdk_get_numa_id, \ .set_etheraddr = netdev_dpdk_set_etheraddr, \ .get_etheraddr = netdev_dpdk_get_etheraddr, \ @@ -6459,6 +6506,7 @@ static const struct netdev_class dpdk_class = { .type = "dpdk", NETDEV_DPDK_CLASS_BASE, .construct = netdev_dpdk_construct, + .get_config = netdev_dpdk_get_config, .set_config = netdev_dpdk_set_config, .send = netdev_dpdk_eth_send, }; @@ -6485,6 +6533,7 @@ static const struct netdev_class dpdk_vhost_client_class = { .init = netdev_dpdk_vhost_class_init, .construct = netdev_dpdk_vhost_client_construct, .destruct = netdev_dpdk_vhost_destruct, + .get_config = netdev_dpdk_vhost_client_get_config, .set_config = netdev_dpdk_vhost_client_set_config, .send = netdev_dpdk_vhost_send, .get_carrier = netdev_dpdk_vhost_get_carrier, diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at index 0f58e8574..fd42aed0b 100644 --- a/tests/system-dpdk.at +++ b/tests/system-dpdk.at @@ -588,8 +588,9 @@ AT_CHECK([ovs-vsctl show], [], [stdout]) sleep 2 dnl Check default MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=1500' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +1500 +]) dnl Increase MTU value and check in the datapath AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9000]) @@ -600,8 +601,9 @@ AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch dnl Fail if error is encountered during MTU setup AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout]) -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +9000 +]) dnl Clean up @@ -636,14 +638,16 @@ dnl Fail if error is encountered during MTU setup AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +9000 +]) dnl Decrease MTU value and check in the datapath AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=2000]) -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=2000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +2000 +]) dnl Clean up @@ -686,16 +690,19 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) dnl Check default MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=1500' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +1500 +]) dnl Increase MTU value and check in the datapath AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9000]) -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +9000 +]) dnl Clean up the testpmd now pkill -f -x -9 'tail -f /dev/null' @@ -743,16 +750,19 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +9000 +]) dnl Decrease MTU value and check in the datapath AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=2000]) -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=2000' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +2000 +]) dnl Clean up the testpmd now pkill -f -x -9 'tail -f /dev/null' @@ -789,8 +799,9 @@ dnl Fail if error is encountered during MTU setup AT_FAIL_IF([grep "Interface phy0 MTU (9702) setup error" ovs-vswitchd.log], [], [stdout]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9702' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +9702 +]) dnl Set MTU value above upper bound and check for error AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9711]) @@ -830,8 +841,9 @@ dnl Fail if error is encountered during MTU setup AT_FAIL_IF([grep "Interface phy0 MTU (68) setup error" ovs-vswitchd.log], [], [stdout]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=68' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl +68 +]) dnl Set MTU value below lower bound and check for error AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=67]) @@ -877,10 +889,12 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=9702' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +9702 +]) dnl Set MTU value above upper bound and check for error AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9711]) @@ -934,10 +948,12 @@ tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\ --single-file-segments -- -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log]) +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 link_state | grep -w up]) dnl Check MTU value in the datapath -AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) -AT_CHECK([grep -E 'mtu=68' stdout], [], [stdout]) +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl +68 +]) dnl Set MTU value below lower bound and check for error AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=67]) diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index d84260d75..87674f088 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -3789,6 +3789,18 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ Maximum number of VMDq pools. </column> + <column name="status" key="n_rxq"> + Number of rx queues. + </column> + + <column name="status" key="n_txq"> + Number of tx queues. + </column> + + <column name="status" key="rx_csum_offload"> + Whether Rx Checksum offload is enabled or not. + </column> + <column name="status" key="if_type"> Interface type ID according to IANA ifTYPE MIB definitions. </column> @@ -3807,7 +3819,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ VF representors. </column> - <column name="status" key="rx_steering"> + <column name="status" key="rx-steering"> Hardware Rx queue steering policy in use. </column>