[ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

Darrell Ball dball at vmware.com
Sat Jul 8 21:33:50 UTC 2017


So, this required a 16.11.2 bug fix.

Are we sure this will work for all/most NICs/drivers ?
In previous alluding to error cases, did calling rte_eth_dev_set_mtu() block jumbo packets
that would otherwise be allowed ?  



On 7/7/17, 3:18 AM, "ovs-dev-bounces at openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces at openvswitch.org on behalf of mark.b.kavanagh at intel.com> wrote:

    >From: Chandran, Sugesh
    >Sent: Friday, July 7, 2017 10:44 AM
    >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; ovs-dev at openvswitch.org;
    >Varghese, Vipin <vipin.varghese at intel.com>; aconole at redhat.com
    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >
    >Hi Mark,
    >Thank you for the information.
    >Please find my comments below.
    >
    >Regards
    >_Sugesh
    >
    >
    >> -----Original Message-----
    >> From: Kavanagh, Mark B
    >> Sent: Wednesday, July 5, 2017 10:20 AM
    >> To: Chandran, Sugesh <sugesh.chandran at intel.com>; ovs-
    >> dev at openvswitch.org; Varghese, Vipin <vipin.varghese at intel.com>;
    >> aconole at redhat.com
    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >>
    >>
    >>
    >> >From: Chandran, Sugesh
    >> >Sent: Monday, July 3, 2017 4:36 PM
    >> >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>;
    >> >ovs-dev at openvswitch.org; Varghese, Vipin <vipin.varghese at intel.com>;
    >> >aconole at redhat.com
    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >> >
    >> >Hi Mark,
    >> >
    >> >Regards
    >> >_Sugesh
    >> >
    >> >
    >> >> -----Original Message-----
    >> >> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
    >> >> bounces at openvswitch.org] On Behalf Of Mark Kavanagh
    >> >> Sent: Wednesday, June 28, 2017 3:52 PM
    >> >> To: ovs-dev at openvswitch.org; Varghese, Vipin
    >> >> <vipin.varghese at intel.com>; aconole at redhat.com
    >> >> Subject: [ovs-dev] [PATCH v3] 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 at redhat.com>
    >> >> Reported-by: Vipin Varghese <vipin.varghese at intel.com>
    >> >> Reviewed-by: Aaron Conole <aconole at redhat.com>
    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
    >> >> ---
    >> >>
    >> >> v3->v2:
    >> >>     - enable scatter_rx explicitly for jumbo MTU
    >> >>
    >> >> v2->v1:
    >> >>     - add 'reported-by' tag for Aaron Conole
    >> >>         - change VLOG_INFO to VLOG_ERR
    >> >>
    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)
    >> >>
    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
    >> >> bba4de3..671d585
    >> >> 100644
    >> >> --- a/lib/netdev-dpdk.c
    >> >> +++ b/lib/netdev-dpdk.c
    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
    >> netdev_dpdk
    >> >> *dev, int n_rxq, int n_txq)
    >> >>      int i;
    >> >>      struct rte_eth_conf conf = port_conf;
    >> >>
    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
    >explicitly
    >> >> +     * enabled. */
    >> >>      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.enable_scatter = 1;
    >> >>      }
    >> >> +
    >> >>      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 @@
    >> >> -676,6 +675,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));
    >> >[Sugesh] I am still getting this error when I try to test this feature
    >> >on a Niantic cards.
    >>
    >> Hi Sugesh,
    >>
    >> What version of DPDK are you using?
    >>
    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2, since
    >this
    >> contains a bugfix for the ixgbe set_mtu function that is required for this
    >> patch.
    >>
    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you can
    >add
    >'Tested by'
    >And the changes are looks ok for me too.
    
    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in that case?
    
    Cheers,
    Mark
    
    >
    >> In any event, I've been in contact with DPDK colleagues wrt the inconsistent
    >> behavior between ixgbe and i40e drivers that warrant this change; they've
    >> pushed a patch to address same:
    >> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .
    >> I haven't had a chance to test this patch yet, but if it works, it may not
    >be
    >> necessary to set scatter_rx mode in OvS.
    >>
    >> Thanks,
    >> Mark
    >>
    >>
    >> >
    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
    >> >mtu_request=5000 2017-07-
    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
    >> >dpdk0 MTU (5000) setup
    >> >error: Invalid argument
    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
    >> txq:2)
    >> >configure error: Invalid argument
    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
    >> >dpdk0 new configuration
    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
    >> >nonexistent port 1
    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
    >> on
    >> >nonexistent port 1
    >> sugeshch at silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
    >> >2017-07-
    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port
    >> >1
    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port 1
    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port 1
    >> >
    >> >I assume just the scatter_flag may not sufficient to enable jumbo_frames??
    >> >Because in my testing I noticed the behavior is same that reported by
    >> >Ian in the v2 patch. No traffic is getting forwarded through the ports.
    >> >
    >> >
    >> >> +            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);
    >> >> --
    >> >> 1.9.3
    >> >>
    >> >> _______________________________________________
    >> >> dev mailing list
    >> >> dev at openvswitch.org
    >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-8bybqH21vcc86SOLmLk&e= 
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-8bybqH21vcc86SOLmLk&e= 
    



More information about the dev mailing list