Hi Ian, Thanks for the review and the updates. It's fine with me.
Regards, Robert > -----Original Message----- > From: Stokes, Ian [mailto:[email protected]] > Sent: Thursday, May 10, 2018 4:20 PM > To: Róbert Mulik <[email protected]>; [email protected] > Subject: RE: [ovs-dev] [PATCH v7] Configurable Link State Change (LSC) > detection mode > > > It is possible to set LSC detection mode to polling or interrupt mode for > > DPDK interfaces. The default is polling mode. To set interrupt mode, > > option dpdk-lsc-interrupt has to be set to true. > > > > For detailed description and usage see the dpdk install documentation. > > > > Thanks Robert, LGTM, > > I've made the following incremental > > diff --git a/NEWS b/NEWS > index 39f42c5..269faea 100644 > --- a/NEWS > +++ b/NEWS > @@ -27,6 +27,7 @@ Post-v2.9.0 > - DPDK: > * New 'check-dpdk' Makefile target to run a new system testsuite. > See Testing topic for the details. > + * Add support for LSC interrupt mode for DPDK devices. > > I've also moved the documentation you added from > > 'Documentation/intro/install/dpdk.rst' to > 'Documentation/topics/dpdk/phy.rst' as I think the proper location for the > configuration options. > > If this is ok with you I'll put this on DPDK_MERGE, I believe we said it > should > be backported also to OVS 2.9 and OVS 2.8 at least. > > Thanks > Ian > > > Signed-off-by: Robert Mulik <[email protected]> > > --- > > v5 -> v6: > > - DPDK install documentation updated. > > - Status of lsc_interrupt_mode of DPDK interfaces can be read by > command > > ovs-appctl dpif/show. > > - It was suggested to check if the HW supports interrupt mode, but it is > > not > > possible to do without DPDK code change, so it is skipped from this > > patch. > > V6 -> v7 > > - DPDK install documentation updated. > > - Commit message updated. > > - Error message updated with LSC failure. > > - Code refactorized based on the comments for v6. > > --- > > Documentation/intro/install/dpdk.rst | 24 ++++++++++++++++++++++++ > > lib/netdev-dpdk.c | 33 ++++++++++++++++++++++++++++--- > > -- > > vswitchd/vswitch.xml | 17 +++++++++++++++++ > > 3 files changed, 69 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/intro/install/dpdk.rst > > b/Documentation/intro/install/dpdk.rst > > index fea4890..c47ea13 100644 > > --- a/Documentation/intro/install/dpdk.rst > > +++ b/Documentation/intro/install/dpdk.rst > > @@ -628,6 +628,30 @@ The average number of packets per output batch > can be > > checked in PMD stats:: > > > > $ ovs-appctl dpif-netdev/pmd-stats-show > > > > +Link State Change (LSC) detection configuration > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > + > > +There are two methods to get the information when Link State Change > > +(LSC) happens on a network interface: by polling or interrupt. > > + > > +Configuring the lsc detection mode has no direct effect on OVS itself, > > +instead it configures the NIC how it should handle link state changes. > > +Processing the link state update request triggered by OVS takes less > > +time using interrupt mode, since the NIC updates its link state in the > > +background, while in polling mode the link state has to be fetched from > > +the firmware every time to fulfil this request. > > + > > +Note that not all PMD drivers support LSC interrupts. > > + > > +The default configuration is polling mode. To set interrupt mode, > > +option ``dpdk-lsc-interrupt`` has to be set to ``true``. > > + > > +Command to set interrupt mode for a specific interface:: > > + $ ovs-vsctl set interface <iface_name> > > +options:dpdk-lsc-interrupt=true > > + > > +Command to set polling mode for a specific interface:: > > + $ ovs-vsctl set interface <iface_name> > > +options:dpdk-lsc-interrupt=false > > + > > Limitations > > ------------ > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ee39cbe..c2ec463 > > 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -434,6 +434,12 @@ struct netdev_dpdk { > > /* DPDK-ETH hardware offload features, > > * from the enum set 'dpdk_hw_ol_features' */ > > uint32_t hw_ol_features; > > + > > + /* Properties for link state change detection mode. > > + * If lsc_interrupt_mode is set to false, poll mode is used, > > + * otherwise interrupt mode is used. */ > > + bool requested_lsc_interrupt_mode; > > + bool lsc_interrupt_mode; > > ); > > > > PADDED_MEMBERS(CACHE_LINE_SIZE, > > @@ -689,12 +695,14 @@ dpdk_watchdog(void *dummy OVS_UNUSED) } > > > > static int > > -dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int > n_txq) > > +dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int > n_txq) > > { > > int diag = 0; > > int i; > > struct rte_eth_conf conf = port_conf; > > > > + conf.intr_conf.lsc = dev->lsc_interrupt_mode; > > + > > /* For some NICs (e.g. Niantic), scatter_rx mode needs to be > > explicitly > > * enabled. */ > > if (dev->mtu > ETHER_MTU) { > > @@ -804,10 +812,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > > n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq); > > n_txq = MIN(info.max_tx_queues, dev->up.n_txq); > > > > - diag = dpdk_eth_dev_queue_setup(dev, n_rxq, n_txq); > > + diag = dpdk_eth_dev_port_config(dev, n_rxq, n_txq); > > if (diag) { > > - VLOG_ERR("Interface %s(rxq:%d txq:%d) configure error: %s", > > - dev->up.name, n_rxq, n_txq, rte_strerror(-diag)); > > + VLOG_ERR("Interface %s(rxq:%d txq:%d lsc interrupt mode:%s) " > > + "configure error: %s", > > + dev->up.name, n_rxq, n_txq, > > + dev->lsc_interrupt_mode ? "true" : "false", > > + rte_strerror(-diag)); > > return -diag; > > } > > > > @@ -900,6 +911,7 @@ common_construct(struct netdev *netdev, > dpdk_port_t > > port_no, > > dev->flags = 0; > > dev->requested_mtu = ETHER_MTU; > > dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu); > > + dev->requested_lsc_interrupt_mode = 0; > > ovsrcu_index_init(&dev->vid, -1); > > dev->vhost_reconfigured = false; > > dev->attached = false; > > @@ -1324,6 +1336,8 @@ netdev_dpdk_get_config(const struct netdev > *netdev, > > struct smap *args) > > } else { > > smap_add(args, "rx_csum_offload", "false"); > > } > > + smap_add(args, "lsc_interrupt_mode", > > + dev->lsc_interrupt_mode ? "true" : "false"); > > } > > ovs_mutex_unlock(&dev->mutex); > > > > @@ -1447,7 +1461,7 @@ netdev_dpdk_set_config(struct netdev *netdev, > const > > struct smap *args, > > char **errp) > > { > > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > > - bool rx_fc_en, tx_fc_en, autoneg; > > + bool rx_fc_en, tx_fc_en, autoneg, lsc_interrupt_mode; > > enum rte_eth_fc_mode fc_mode; > > static const enum rte_eth_fc_mode fc_mode_set[2][2] = { > > {RTE_FC_NONE, RTE_FC_TX_PAUSE}, > > @@ -1524,6 +1538,12 @@ netdev_dpdk_set_config(struct netdev > *netdev, const > > struct smap *args, > > goto out; > > } > > > > + lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", > > false); > > + if (dev->requested_lsc_interrupt_mode != lsc_interrupt_mode) { > > + dev->requested_lsc_interrupt_mode = lsc_interrupt_mode; > > + netdev_request_reconfigure(netdev); > > + } > > + > > rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false); > > tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false); > > autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); @@ -3554,6 > > +3574,7 @@ netdev_dpdk_reconfigure(struct netdev *netdev) > > if (netdev->n_txq == dev->requested_n_txq > > && netdev->n_rxq == dev->requested_n_rxq > > && dev->mtu == dev->requested_mtu > > + && dev->lsc_interrupt_mode == dev- > >requested_lsc_interrupt_mode > > && dev->rxq_size == dev->requested_rxq_size > > && dev->txq_size == dev->requested_txq_size > > && dev->socket_id == dev->requested_socket_id) { @@ -3569,6 > > +3590,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev) > > goto out; > > } > > > > + dev->lsc_interrupt_mode = dev->requested_lsc_interrupt_mode; > > + > > netdev->n_txq = dev->requested_n_txq; > > netdev->n_rxq = dev->requested_n_rxq; > > > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index > > 9c2a826..aeb1a47 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -3627,6 +3627,23 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > > type=patch options:peer=p1 \ > > </column> > > </group> > > > > + <group title="Link State Change detection mode"> > > + <column name="options" key="dpdk-lsc-interrupt" > > + type='{"type": "boolean"}'> > > + <p> > > + Set this value to <code>true</code> to configure interrupt mode > > for > > + Link State Change (LSC) detection instead of poll mode for the > > DPDK > > + interface. > > + </p> > > + <p> > > + If this value is not set, poll mode is configured. > > + </p> > > + <p> > > + This parameter has an effect only on netdev dpdk interfaces. > > + </p> > > + </column> > > + </group> > > + > > <group title="Common Columns"> > > The overall purpose of these columns is described under > > <code>Common > > Columns</code> at the beginning of this document. > > -- > > 1.9.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
