> On Fri, Nov 24, 2017 at 9:26 AM, Stokes, Ian <[email protected]> wrote:
> >> Some PMD assumes that the RX buffers are already allocated when
> >> setting the device MTU, because the RX buffer size depends on the MTU.
> >> This worked until 67fe6d635193 added a call to rte_eth_dev_set_mtu()
> >> in the init code, which would set the MTU before the RX buffer
> >> allocation, triggering a segmentation fault with some PMD:
> >>
> >> Stack trace of thread 20680:
> >> #0 0x0000559464396534 qede_set_mtu (ovs-vswitchd)
> >> #1 0x0000559464323c41 rte_eth_dev_set_mtu (ovs-vswitchd)
> >> #2 0x00005594645f5e85 dpdk_eth_dev_queue_setup (ovs-vswitchd)
> >> #3 0x00005594645f8ae6 netdev_dpdk_reconfigure (ovs-vswitchd)
> >> #4 0x000055946452225c reconfigure_datapath (ovs-vswitchd)
> >> #5 0x0000559464522d07 do_add_port (ovs-vswitchd)
> >> #6 0x0000559464522e8d dpif_netdev_port_add (ovs-vswitchd)
> >> #7 0x0000559464528dde dpif_port_add (ovs-vswitchd)
> >> #8 0x00005594644dc0e0 port_add (ovs-vswitchd)
> >> #9 0x00005594644d2ab1 ofproto_port_add (ovs-vswitchd)
> >> #10 0x00005594644c0a85 bridge_add_ports__ (ovs-vswitchd)
> >> #11 0x00005594644c26e8 bridge_reconfigure (ovs-vswitchd)
> >> #12 0x00005594644c5c49 bridge_run (ovs-vswitchd)
> >> #13 0x00005594642d4155 main (ovs-vswitchd)
> >> #14 0x00007f0e1444bc05 __libc_start_main (libc.so.6)
> >> #15 0x00005594642d8328 _start (ovs-vswitchd)
> >>
> >> A possible solution could be to move the first call to
> >> rte_eth_dev_set_mtu() just after the device start instead of
> >> dpdk_eth_dev_queue_setup() which, by the way, set the MTU multiple
> >> times as the call to rte_eth_dev_set_mtu() was in a loop.
> >>
> >> CC: Mark Kavanagh <[email protected]>
> >> Fixes: 67fe6d635193 ("netdev-dpdk: use rte_eth_dev_set_mtu.")
> >> Signed-off-by: Matteo Croce <[email protected]>
> >> ---
> >> 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
> >> 76e79be25..229aa4a76 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -750,13 +750,6 @@ 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); @@
> >> -
> >> 849,6 +842,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> >> return -diag;
> >> }
> >>
> >> + 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));
> >> + return -diag;
> >> + }
> >> +
> >
> > In my testing I didn't see any issues here, but would like to flag it to
> our MTU Guru (Mark K).
> >
> > Any opinion on this? From the API description it looks like it is ok?
> >
> >
> >> rte_eth_promiscuous_enable(dev->port_id);
> >> rte_eth_allmulticast_enable(dev->port_id);
> >>
> >> --
> >> 2.13.6
> >>
> >> _______________________________________________
> >> dev mailing list
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> The issue only arises with the qede PMD and 67fe6d635193
> ("netdev-dpdk: use rte_eth_dev_set_mtu.")
I had some more time to look at this today but this patch will break OVS DPDK
for existing supported DPDK ports during testing.
I tested with an XL710 and the MTU will fail to apply, the device must be
stopped before configuration changes can be applied such as MTU. See log
message below
2017-11-28T17:13:50Z|00086|dpdk|ERR|i40e_dev_mtu_set(): port 0 must be stopped
before configuration
2017-11-28T17:13:50Z|00087|netdev_dpdk|ERR|Interface dpdk0 MTU (1500) setup
error: Device or resource busy
Did you come across it in your testing? I guess you didn’t see this for QEDE
pmd. In my case the DPDK devices will simply fail to add to the bridge.
As is, the patch would not be acceptable as its breaking existing
functionality. It would have to be reworked to configure for device that cannot
reconfigure when busy.
Thanks
Ian
>
> Regards,
> --
> Matteo Croce
> per aspera ad upstream
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
