On 28/02/18 15:23, Róbert Mulik wrote:
-----Original Message-----
From: Eelco Chaudron [mailto:[email protected]]
Sent: Wednesday, February 21, 2018 1:29 PM
To: Róbert Mulik <[email protected]>; [email protected]
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: [email protected] [mailto:ovs-dev-
[email protected]] On Behalf Of Eelco Chaudron
Sent: Tuesday, February 20, 2018 3:17 PM
To: [email protected]
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?
Did not look at the testpmd code close enough, but looking at it again
it seems they use a hack by accessing some data directly :(
2085 <http://127.0.0.1:8080/source/xref/dpdk/app/test-pmd/testpmd.c#2085> *if* (lsc_interrupt
<http://127.0.0.1:8080/source/xref/dpdk/app/test-pmd/testpmd.c#lsc_interrupt> &&
2086 <http://127.0.0.1:8080/source/xref/dpdk/app/test-pmd/testpmd.c#2086> (rte_eth_devices
<http://127.0.0.1:8080/source/s?defs=rte_eth_devices&project=dpdk>[pid <http://127.0.0.1:8080/source/s?defs=pid&project=dpdk>].data <http://127.0.0.1:8080/source/s?defs=data&project=dpdk>->dev_flags <http://127.0.0.1:8080/source/s?defs=dev_flags&project=dpdk> &
2087 <http://127.0.0.1:8080/source/xref/dpdk/app/test-pmd/testpmd.c#2087> RTE_ETH_DEV_INTR_LSC
<http://127.0.0.1:8080/source/s?defs=RTE_ETH_DEV_INTR_LSC&project=dpdk>))
2088 <http://127.0.0.1:8080/source/xref/dpdk/app/test-pmd/testpmd.c#2088> port
<http://127.0.0.1:8080/source/s?defs=port&project=dpdk>->dev_conf
<http://127.0.0.1:8080/source/s?defs=dev_conf&project=dpdk>.intr_conf
<http://127.0.0.1:8080/source/s?defs=intr_conf&project=dpdk>.lsc
<http://127.0.0.1:8080/source/s?defs=lsc&project=dpdk> =1;
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 <[email protected]>
---
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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
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
