diff mbox

[ovs-dev,V2] netdev-dpdk: use rte_eth_dev_set_mtu

Message ID 1497265522-32549-1-git-send-email-mark.b.kavanagh@intel.com
State Superseded
Headers show

Commit Message

Mark Kavanagh June 12, 2017, 11:05 a.m. UTC
DPDK provides an API to set the MTU of compatible physical devices -
rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
implemented in some DPDK PMDs (i40e, specifically). To allow the use
of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
achieved by setting the jumbo frame flag, and corresponding maximum
permitted Rx frame size, in an rte_eth_conf structure for the NIC
port, and subsequently invoking rte_eth_dev_configure() with that
configuration.

However, that method does not set the MTU field of the underlying DPDK
structure (rte_eth_dev) for the corresponding physical device;
consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
OvS-DPDK phy device with non-standard MTU.

Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
or modifying the MTU of a DPDK phy port.

Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
Reported-by: Aaron Conole <aconole@redhat.com>
Reported-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
---

V2->v1:
    - add 'reported-by' tag for Aaron Conole
    - change VLOG_INFO to VLOG_ERR

 lib/netdev-dpdk.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Aaron Conole June 13, 2017, 7:08 p.m. UTC | #1
Mark Kavanagh <mark.b.kavanagh@intel.com> writes:

> DPDK provides an API to set the MTU of compatible physical devices -
> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
> implemented in some DPDK PMDs (i40e, specifically). To allow the use
> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
> achieved by setting the jumbo frame flag, and corresponding maximum
> permitted Rx frame size, in an rte_eth_conf structure for the NIC
> port, and subsequently invoking rte_eth_dev_configure() with that
> configuration.
>
> However, that method does not set the MTU field of the underlying DPDK
> structure (rte_eth_dev) for the corresponding physical device;
> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
> OvS-DPDK phy device with non-standard MTU.
>
> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
> or modifying the MTU of a DPDK phy port.
>
> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
> Reported-by: Aaron Conole <aconole@redhat.com>
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> ---

I won't have a chance to test this near-term, but it looks correct to
me.  Thanks for this work, Mark!

Reviewed-by: Aaron Conole <aconole@redhat.com>
Stokes, Ian June 23, 2017, 10:11 a.m. UTC | #2
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
> Sent: Monday, June 12, 2017 12:05 PM
> To: ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
> aconole@redhat.com
> Subject: [ovs-dev] [PATCH V2] netdev-dpdk: use rte_eth_dev_set_mtu
> 
> DPDK provides an API to set the MTU of compatible physical devices -
> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
> implemented in some DPDK PMDs (i40e, specifically). To allow the use of
> jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
> achieved by setting the jumbo frame flag, and corresponding maximum
> permitted Rx frame size, in an rte_eth_conf structure for the NIC port,
> and subsequently invoking rte_eth_dev_configure() with that configuration.
> 
> However, that method does not set the MTU field of the underlying DPDK
> structure (rte_eth_dev) for the corresponding physical device;
> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an OvS-
> DPDK phy device with non-standard MTU.
> 
> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up or
> modifying the MTU of a DPDK phy port.
> 
> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
> Reported-by: Aaron Conole <aconole@redhat.com>
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> ---
> 
> V2->v1:
>     - add 'reported-by' tag for Aaron Conole
>     - change VLOG_INFO to VLOG_ERR
> 
>  lib/netdev-dpdk.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 810800e..c141dc2
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -649,13 +649,6 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
> n_rxq, int n_txq)
>      int i;
>      struct rte_eth_conf conf = port_conf;
> 
> -    if (dev->mtu > ETHER_MTU) {
> -        conf.rxmode.jumbo_frame = 1;
> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
> -    } else {
> -        conf.rxmode.jumbo_frame = 0;
> -        conf.rxmode.max_rx_pkt_len = 0;
> -    }
>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>      /* A device may report more queues than it makes available (this has
> @@ -675,6 +668,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
> n_rxq, int n_txq)
>              break;
>          }
> 
> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
> +        if (diag) {
> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
> +            break;
> +        }
> +
>          for (i = 0; i < n_txq; i++) {
>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
>                                            dev->socket_id, NULL);
> --

Hi Mark,

I've tested this patch with both an Intel X710 and an Intel 82599ES. The X710 seems fine however for the 82599ES I found that setting an MTU greater than 1500 resulted in the errors below and the port was left in a down state no longer able to pass traffic.

2017-06-23T09:47:49Z|00089|netdev_dpdk|ERR|Interface dpdk0 MTU (9000) setup error: Invalid argument
2017-06-23T09:47:49Z|00090|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2) configure error: Invalid argument
2017-06-23T09:47:49Z|00091|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration
2017-06-23T09:47:49Z|00092|ofproto|WARN|br0: cannot get STP status on nonexistent port 1
2017-06-23T09:47:49Z|00093|ofproto|WARN|br0: cannot get RSTP status on nonexistent port 1

I could not reproduce the issue once I reverted the patch so it seems specific to the use of rte_eth_dev_set_mtu() with the 82599ES.

Ian

> 1.9.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Mark Kavanagh June 23, 2017, 10:33 a.m. UTC | #3
>From: Stokes, Ian
>Sent: Friday, June 23, 2017 11:11 AM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin
><vipin.varghese@intel.com>; aconole@redhat.com
>Subject: RE: [ovs-dev] [PATCH V2] netdev-dpdk: use rte_eth_dev_set_mtu
>
>
>
>> -----Original Message-----
>> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
>> Sent: Monday, June 12, 2017 12:05 PM
>> To: ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
>> aconole@redhat.com
>> Subject: [ovs-dev] [PATCH V2] netdev-dpdk: use rte_eth_dev_set_mtu
>>
>> DPDK provides an API to set the MTU of compatible physical devices -
>> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
>> implemented in some DPDK PMDs (i40e, specifically). To allow the use of
>> jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
>> achieved by setting the jumbo frame flag, and corresponding maximum
>> permitted Rx frame size, in an rte_eth_conf structure for the NIC port,
>> and subsequently invoking rte_eth_dev_configure() with that configuration.
>>
>> However, that method does not set the MTU field of the underlying DPDK
>> structure (rte_eth_dev) for the corresponding physical device;
>> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an OvS-
>> DPDK phy device with non-standard MTU.
>>
>> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up or
>> modifying the MTU of a DPDK phy port.
>>
>> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
>> Reported-by: Aaron Conole <aconole@redhat.com>
>> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> ---
>>
>> V2->v1:
>>     - add 'reported-by' tag for Aaron Conole
>>     - change VLOG_INFO to VLOG_ERR
>>
>>  lib/netdev-dpdk.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 810800e..c141dc2
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -649,13 +649,6 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
>> n_rxq, int n_txq)
>>      int i;
>>      struct rte_eth_conf conf = port_conf;
>>
>> -    if (dev->mtu > ETHER_MTU) {
>> -        conf.rxmode.jumbo_frame = 1;
>> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
>> -    } else {
>> -        conf.rxmode.jumbo_frame = 0;
>> -        conf.rxmode.max_rx_pkt_len = 0;
>> -    }
>>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>      /* A device may report more queues than it makes available (this has
>> @@ -675,6 +668,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
>> n_rxq, int n_txq)
>>              break;
>>          }
>>
>> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
>> +        if (diag) {
>> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
>> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
>> +            break;
>> +        }
>> +
>>          for (i = 0; i < n_txq; i++) {
>>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
>>                                            dev->socket_id, NULL);
>> --
>
>Hi Mark,
>
>I've tested this patch with both an Intel X710 and an Intel 82599ES. The X710 seems fine
>however for the 82599ES I found that setting an MTU greater than 1500 resulted in the errors
>below and the port was left in a down state no longer able to pass traffic.
>
>2017-06-23T09:47:49Z|00089|netdev_dpdk|ERR|Interface dpdk0 MTU (9000) setup error: Invalid
>argument
>2017-06-23T09:47:49Z|00090|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2) configure error:
>Invalid argument
>2017-06-23T09:47:49Z|00091|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration
>2017-06-23T09:47:49Z|00092|ofproto|WARN|br0: cannot get STP status on nonexistent port 1
>2017-06-23T09:47:49Z|00093|ofproto|WARN|br0: cannot get RSTP status on nonexistent port 1
>
>I could not reproduce the issue once I reverted the patch so it seems specific to the use of
>rte_eth_dev_set_mtu() with the 82599ES.

Hey Ian,

Thanks for testing this patch; I actually became aware of this issue yesterday evening, when Sugesh alerted me to the same issue with Niantic.

As you're probably aware, each NIC has its own set of callback functions, each of which is invoked in response to an rte_eth_dev_xyz function call.
In this case, the rte_eth_dev_set_mtu function invokes ixgbe_dev_mtu_set, which checks if scattered_rx has previously been enabled for the NIC; if not, as is the case here, the MTU is refused.

The solution is most likely to enable scattered_rx in the rte_eth_dev's eth_conf.rx_mode - in any event, I'll include the fix in v3, which I'll push next week.

Thanks again,
Mark

>
>Ian
>
>> 1.9.3
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 810800e..c141dc2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -649,13 +649,6 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
     int i;
     struct rte_eth_conf conf = port_conf;
 
-    if (dev->mtu > ETHER_MTU) {
-        conf.rxmode.jumbo_frame = 1;
-        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
-    } else {
-        conf.rxmode.jumbo_frame = 0;
-        conf.rxmode.max_rx_pkt_len = 0;
-    }
     conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
                                   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
     /* A device may report more queues than it makes available (this has
@@ -675,6 +668,13 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
             break;
         }
 
+        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
+        if (diag) {
+            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
+                    dev->up.name, dev->mtu, rte_strerror(-diag));
+            break;
+        }
+
         for (i = 0; i < n_txq; i++) {
             diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
                                           dev->socket_id, NULL);