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

Kavanagh, Mark B mark.b.kavanagh at intel.com
Wed Jul 12 08:04:09 UTC 2017


>From: Darrell Ball [mailto:dball at vmware.com]
>Sent: Tuesday, July 11, 2017 5:03 PM
>To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; 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
>
>
>
>On 7/11/17, 8:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com> wrote:
>
>    >From: Darrell Ball [mailto:dball at vmware.com]
>    >Sent: Tuesday, July 11, 2017 2:03 AM
>    >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; 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
>    >
>    >
>    >
>    >On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com>
>wrote:
>    >
>    >    >From: Darrell Ball [mailto:dball at vmware.com]
>    >    >Sent: Saturday, July 8, 2017 10:34 PM
>    >    >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; 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
>    >    >
>    >    >So, this required a 16.11.2 bug fix.
>    >    >
>    >    >Are we sure this will work for all/most NICs/drivers ?
>    >
>    >    I can't say tbh Darrell, as I can only test with the NICs that are
>    >available to me (namely, Niantic and Fortville).
>    >
>    >    However, AFAIA, this issue is unique to Niantic, since that H/W
>permits
>    >dynamic modification of MTU without stopping its PMD; for most NICs
>(including
>    >Fortville), a port's PMD must be stopped before modifying its MTU, and
>    >restarted thereafter.
>    >
>    >I understand
>    >
>    >
>    >    >In previous alluding to error cases, did calling
>rte_eth_dev_set_mtu()
>    >block
>    >    >jumbo packets
>    >    >that would otherwise be allowed ?
>    >
>    >    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would
>    >return EINVAL, which means that the MTU for the NIC's ports could never
>be
>    >changed beyond the standard MTU.
>    >
>    >    With this patch, that behavior is avoided, and the MTU can be set
>    >appropriately.
>    >
>    >
>    >The question that I was getting at is since we are now removing the
>    >jumbo_frame flag workaround with this patch, can there be a regression
>where
>    >we cannot
>    >forward jumbo frames because rte_eth_dev_set_mtu() fails due to still
>another
>    >bug in some
>    >cases ?
>
>    Sure, I definitely understand your concerns Darrel - unfortunately, I
>can't definitively answer yes. However, since OvS now supports DPDK v16.11.2,
>I would hope that the stability of the latter would provide some level of
>assurance.
>
>    >If the answer is yes, does it make sense to keep the jumbo_frame flag
>    >workaround as well,
>    >‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle
>unknown
>    >corner cases ?
>
>    If we keep the workaround, it essentially negates the need for the
>netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx
>packet length are set by rte_eth_dev_configure. The downside of this approach
>is that it doesn't update the MTU for the Ethernet device itself; as a result,
>any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the
>MTU that was set using the existing method.
>
>Let us just go ahead with the patch and fix with any potential fallout/bugs,
>as it is the right approach
>anyways.

Thanks Darrell - much appreciated.

I'm, spinning a v4 to add 'acked-by' and 'tested-by' tags for Sugesh - shall I add an 'acked-by' for you also?

Thanks again,
Mark



>
>    Thanks again,
>    Mark
>
>    >
>    >
>    >    >
>    >    >
>    >    >
>    >    >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=uilaK90D4TOVoH58JNXRg
>Q
>    >&
>    >    >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