[ovs-dev] [PATCH v5] Configurable Link State Change (LSC) detection mode
Róbert Mulik
robert.mulik at ericsson.com
Wed Feb 28 14:23:49 UTC 2018
> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro at redhat.com]
> Sent: Wednesday, February 21, 2018 1:29 PM
> To: Róbert Mulik <robert.mulik at ericsson.com>; ovs-dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v5] Configurable Link State Change (LSC)
> detection mode
>
> On 21/02/18 11:33, Róbert Mulik wrote:
> >
> >> -----Original Message-----
> >> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> >> bounces at openvswitch.org] On Behalf Of Eelco Chaudron
> >> Sent: Tuesday, February 20, 2018 3:17 PM
> >> To: ovs-dev at openvswitch.org
> >> Subject: Re: [ovs-dev] [PATCH v5] Configurable Link State Change (LSC)
> >> detection mode
> >>
> >> Hi Robert,
> >>
> >> I did not see a reply to my v4 reply, so I ask this question again:
> >> Did you try what happens if the PMD does not support LSC and its
> >> enabled?
> > Unfortunately I don't have any cards which don't support LSC, so I couldn't
> test it
> Guess you could change the code for the card you have to return an
> error, or not return it as a supported option.
> See my earlier command to get the port supported option and error out
> with an error that the card does not support it.
You suggested to check the flag RTE_ETH_DEV_INTR_LSC to see if the NIC
supports the LSC interrupt mode. I couldn't find a way to check this flag with
the current OVS implementation. As I see the DPDK code has to be changed
to support it. Do you have any suggestion what to do now? Or did I miss
something, and the current implementation can see this flag?
> >> Also, it would be easier to understand what you are going to change if
> >> you answer the questions in the review emails.
> >>
> >> For example, this patch is missing the suggestion on adding the LSC
> >> config in netdev_dpdk_get_config(), as suggested by Ilya.
> > As I understand the reason for this function would be to avoid the
> confusion when
> > both global and local configs are configured for an interface, especially if we
> go with
> > the implementation where the global config doesn't take effect without
> service
> > restart.
> >
> > Since the global config is not implemented in this patch and the default
> value remains
> > the same as it always was (poll mode) and also the documentation tells
> what the default
> > value is, I don't really see the importance of this implementation.
> I have to disagree here, as the getdev_dpdk_get_config() has all the
> configurable options, and you are adding one.
> So please add it.
>
> >> I did a quick visual review below, and will wait for v6 with the above
> >> change, and do another test run.
> >>
> >> //Eelco
> >>
> >>
> >> On 15/02/18 15:44, Róbert Mulik wrote:
> >>> It is possible to change 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.
> >>>
> >>> In polling mode more processor time is needed, since the OVS
> repeatedly
> >> reads
> >>> the link state with a short period. It can lead to packet loss for certain
> >>> systems.
> >>>
> >>> In interrupt mode the hardware itself triggers an interrupt when link
> state
> >>> change happens, so less processing time needs for the OVS.
> >>>
> >>> For detailed description and usage see the dpdk install documentation.
> >>>
> >>> Signed-off-by: Robert Mulik <robert.mulik at ericsson.com>
> >>> ---
> >>> Documentation/intro/install/dpdk.rst | 35
> >> +++++++++++++++++++++++++++++++++++
> >>> lib/netdev-dpdk.c | 22 ++++++++++++++++++++--
> >>> vswitchd/vswitch.xml | 17 +++++++++++++++++
> >>> 3 files changed, 72 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/intro/install/dpdk.rst
> >> b/Documentation/intro/install/dpdk.rst
> >>> index ed358d5..e3872e7 100644
> >>> --- a/Documentation/intro/install/dpdk.rst
> >>> +++ b/Documentation/intro/install/dpdk.rst
> >>> @@ -628,6 +628,41 @@ 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.
> >>> +
> >>> +With the polling method, the main process checks the link state on a
> >>> +fixed interval. This fixed interval determines how fast a link change is
> >>> +detected. Another problem with the poll mode is that on some
> hardware
> >> a
> >>> +polling cycle takes too much time, which (in the end) leads to packet
> loss
> >>> +for certain systems.
> >> Maybe we should stick to facts of the implementation, and not known
> >> bugs? So remove the "Another problem..."
> >>> +
> >>> +If interrupts are used to get LSC information, the hardware itself
> triggers
> >>> +an interrupt when link state change happens, the interrupt thread
> wakes
> >> up
> >>> +from sleep, updates the information, and goes back to sleep mode.
> When
> >> no link
> >>> +state change happens (most of the time), the thread remains in sleep
> >> mode and
> >>> +doesn`t use processor time at all. The disadvantage of this method is
> that
> >>> +when an interrupt happens, the processor has to handle it
> immediately,
> >> so it
> >>> +puts the currently running process to background, handles the
> interrupt,
> >> and
> >>> +takes the background process back.
> >>> +
> >>> +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
> >>> +
> >>> +Command to remove ``dpdk-lsc-interrupt`` option::
> >>> + $ ovs-vsctl remove interface <iface_name> options dpdk-lsc-
> interrupt
> >>> +
> >>> Limitations
> >>> ------------
> >>>
> >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >>> index 94fb163..d092ef1 100644
> >>> --- a/lib/netdev-dpdk.c
> >>> +++ b/lib/netdev-dpdk.c
> >>> @@ -433,6 +433,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,
> >>> @@ -686,12 +692,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) {
> >>> @@ -801,7 +809,7 @@ 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));
> >>> @@ -897,6 +905,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;
> >>> @@ -1520,6 +1529,12 @@ netdev_dpdk_set_config(struct netdev
> >> *netdev, const struct smap *args,
> >>> goto out;
> >>> }
> >>>
> >>> + bool 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);
> >>> @@ -3546,6 +3561,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) {
> >>> @@ -3561,6 +3577,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 0c6a43d..3c9e637 100644
> >>> --- a/vswitchd/vswitch.xml
> >>> +++ b/vswitchd/vswitch.xml
> >>> @@ -3631,6 +3631,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 at openvswitch.org
> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list