[ovs-dev,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.

Signed-off-by: Robert Mulik <robert.mulik@ericsson.com>
---
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(-)

--
1.9.1

> 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 <robert.mulik@ericsson.com>
> ---
> 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
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Hi Ian,

Thanks for the review and the updates. It's fine with me.

Regards,
Robert

> -----Original Message-----
> From: Stokes, Ian [mailto:ian.stokes@intel.com]
> Sent: Thursday, May 10, 2018 4:20 PM
> To: Róbert Mulik <robert.mulik@ericsson.com>; dev@openvswitch.org
> 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 <robert.mulik@ericsson.com>
> > ---
> > 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
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev