> -----Original Message----- > From: Stokes, Ian > Sent: Tuesday, January 9, 2018 10:54 PM > To: [email protected] > Cc: Stokes, Ian <[email protected]>; Kavanagh, Mark B > <[email protected]> > Subject: [RFC PATCH v1 1/1] netdev-dpdk: Fix requested MTU size > validation. > > This commit replaces MTU_TO_FRAME_LEN(mtu) with > MTU_TO_MAX_FRAME_LEN(mtu) when validating if an MTU will exceed > NETDEV_DPDK_MAX_PKT_LEN in netdev_dpdk_set_mtu(). > > When setting an MTU we first check if the requested MTU frame size will > exceed the maximum packet frame size supported in netdev_dpdk_set_mtu(). > The MTU frame length is calculated as MTU + ETHER_HEADER + ETHER_CRC. The > MTU for the device will be set at a later stage in dpdk_ethdev_init() > using rte_ethdev_set_mtu(mtu). > > However when using rte_ethdev_set_mtu(mtu) the calculation used to check > that the MTU does not exceed the max frame length for that device varies > between DPDK device drivers. For example ixgbe driver calculates MTU frame > length as > > mtu + ETHER_HDR_LEN + ETHER_CRC_LEN > > i40e driver calculates it as > > ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2
Apologies folks, typo above, calculation should read: MTU + ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2 Ian > > em driver calculates it as > > mtu + ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE > > Currently it is possible to set an MTU for a netdev_dpdk device that > exceeds the upper limit MTU for that devices DPDK driver. This leads to a > segfault. This is because the MTU frame length comparison as is, does not > take into account the addition of the vlan tag overhead expected in the > drivers. The netdev_dpdk_set_mtu() call will incorrectly succeed but the > subsequent dpdk_ethdev_init() will fail before before queues have be > created for DPDK device. This coupled with assumptions regarding > reconfiguration requirements for the netdev will lead to a segfault when > the rxq is polled for this device. > > A simple way to avoid this is by using MTU_TO_MAX_FRAME_LEN(mtu) when > validating a requested MTU in netdev_dpdk_set_mtu() to account for vlan > overhead required by some devices in MTU frame length. > > Note: this fix is a work around, a better approach would be if DPDK > devices could report the maximum MTU value that can be requested on a per > device basis. This capability however is not currently available. A > downside of this patch is that the MTU upper limit will be reduced by 8 > bytes for DPDK devices that do not need to account for vlan tags in MTU > frame length driver calculations e.g. ixgbe devices upper MTU limit is > reduced from the ovs point of view from 9710 to 9702. > > As this patch is RFC it has limited testing to i40e and ixgbe devices. > If acceptable to the community documentation will also have to be updated > under limitations I expect. > > CC: Mark Kavanagh <[email protected]> > Fixes: 0072e931 ("netdev-dpdk: add support for jumbo frames") > Signed-off-by: Ian Stokes <[email protected]> > --- > lib/netdev-dpdk.c | 11 ++++++++++- > 1 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 364f545..4da5292 > 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1999,7 +1999,16 @@ netdev_dpdk_set_mtu(struct netdev *netdev, int mtu) > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > > - if (MTU_TO_FRAME_LEN(mtu) > NETDEV_DPDK_MAX_PKT_LEN > + /* XXX: We need to ensure the requested MTU frame length does not > + * surpass the NETDEV_DPDK_MAX_PKT_LEN. DPDK device drivers differ > + * in how the MTU frame length is calculated when > rte_ethdev_set_mtu(mtu) > + * is called e.g. i40e and em drivers include vlan tags as part of > the > + * MTU frame length. As such we should use MTU_TO_MAX_FRAME_LEN(mtu) > + * for comparison here to avoid a failure later with > rte_ethdev_set_mtu(). > + * This approach should be used until DPDK provides a method to > retrieve > + * maximum MTU frame length for a given device. > + */ > + if (MTU_TO_MAX_FRAME_LEN(mtu) > NETDEV_DPDK_MAX_PKT_LEN > || mtu < ETHER_MIN_MTU) { > VLOG_WARN("%s: unsupported MTU %d\n", dev->up.name, mtu); > return EINVAL; > -- > 1.7.0.7 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
