From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id EAC17A0353; Mon, 11 Nov 2019 00:07:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 068F3E07; Mon, 11 Nov 2019 00:07:49 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id CD857DE3 for ; Mon, 11 Nov 2019 00:07:45 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Nov 2019 15:07:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,290,1569308400"; d="scan'208";a="201896187" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by fmsmga008.fm.intel.com with ESMTP; 10 Nov 2019 15:07:41 -0800 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 10 Nov 2019 23:07:39 +0000 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.252]) by irsmsx155.ger.corp.intel.com ([169.254.14.193]) with mapi id 14.03.0439.000; Sun, 10 Nov 2019 23:07:39 +0000 From: "Ananyev, Konstantin" To: Dekel Peled , "Mcnamara, John" , "Kovacevic, Marko" , "nhorman@tuxdriver.com" , "ajit.khaparde@broadcom.com" , "somnath.kotur@broadcom.com" , "Burakov, Anatoly" , "xuanziyang2@huawei.com" , "cloud.wangxiaoyun@huawei.com" , "zhouguoyang@huawei.com" , "Lu, Wenzhuo" , "matan@mellanox.com" , "shahafs@mellanox.com" , "viacheslavo@mellanox.com" , "rmody@marvell.com" , "shshaikh@marvell.com" , "maxime.coquelin@redhat.com" , "Bie, Tiwei" , "Wang, Zhihong" , "yongwang@vmware.com" , "thomas@monjalon.net" , "Yigit, Ferruh" , "arybchenko@solarflare.com" , "Wu, Jingjing" , "Iremonger, Bernard" CC: "dev@dpdk.org" Thread-Topic: [PATCH v5 1/3] ethdev: support API to set max LRO packet size Thread-Index: AQHVllPQujwU493i4ECQgVknOV62maeFCHeg Date: Sun, 10 Nov 2019 23:07:39 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725801A8C8578D@IRSMSX104.ger.corp.intel.com> References: In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODQ1ZmU3MWEtMDg1Ni00MmE1LTk4OTMtNGFhYTU0MTZhMDkwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQlwvTEhYV1JjakJ6eVpZWm1xeVQrSUlXMjZ4OGFIOTVoTzhSSGd2RHdBaWVEcWxvTVI1cDRhNk52RVlLOFlldXIifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v5 1/3] ethdev: support API to set max LRO packet size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > This patch implements [1], to support API for configuration and > validation of max size for LRO aggregated packet. > API change notice [2] is removed, and release notes for 19.11 > are updated accordingly. >=20 > Various PMDs using LRO offload are updated, the new data members are > initialized to ensure they don't fail validation. >=20 > [1] http://patches.dpdk.org/patch/58217/ > [2] http://patches.dpdk.org/patch/57492/ Actually if the requirement is just to allow user to limit max lro size, then why not to add just new function for that: int rte_eth_dev_set_max_lro(uint16_t port_id, uint32_t lro); ? And make it optional for the drivers to support it. That way PMD/devices that allow LRO max size to be configurable, can support it others can fail. =20 Konstantin >=20 > Signed-off-by: Dekel Peled > Reviewed-by: Andrew Rybchenko > Acked-by: Thomas Monjalon > Acked-by: Matan Azrad > --- > doc/guides/nics/features.rst | 2 ++ > doc/guides/rel_notes/deprecation.rst | 4 ---- > doc/guides/rel_notes/release_19_11.rst | 8 +++++++ > drivers/net/bnxt/bnxt_ethdev.c | 1 + > drivers/net/hinic/hinic_pmd_ethdev.c | 1 + > drivers/net/ixgbe/ixgbe_ethdev.c | 1 + > drivers/net/mlx5/mlx5.h | 3 +++ > drivers/net/mlx5/mlx5_ethdev.c | 1 + > drivers/net/mlx5/mlx5_rxq.c | 1 - > drivers/net/qede/qede_ethdev.c | 1 + > drivers/net/virtio/virtio_ethdev.c | 1 + > drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 + > lib/librte_ethdev/rte_ethdev.c | 44 ++++++++++++++++++++++++++++= ++++++ > lib/librte_ethdev/rte_ethdev.h | 4 ++++ > 14 files changed, 68 insertions(+), 5 deletions(-) >=20 > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst > index 7a31cf7..2138ce3 100644 > --- a/doc/guides/nics/features.rst > +++ b/doc/guides/nics/features.rst > @@ -193,10 +193,12 @@ LRO > Supports Large Receive Offload. >=20 > * **[uses] rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFL= OAD_TCP_LRO``. > + ``dev_conf.rxmode.max_lro_pkt_size``. > * **[implements] datapath**: ``LRO functionality``. > * **[implements] rte_eth_dev_data**: ``lro``. > * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_LRO``, ``mbuf.tso_segsz`= `. > * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_= capa:DEV_RX_OFFLOAD_TCP_LRO``. > +* **[provides] rte_eth_dev_info**: ``max_lro_pkt_size``. >=20 >=20 > .. _nic_features_tso: > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/= deprecation.rst > index c10dc30..fdec33d 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -87,10 +87,6 @@ Deprecation Notices > This scheme will allow PMDs to avoid lookup to internal ptype table on= Rx and > thereby improve Rx performance if application wishes do so. >=20 > -* ethdev: New 32-bit fields may be added for maximum LRO session size, i= n > - struct ``rte_eth_dev_info`` for the port capability and in struct > - ``rte_eth_rxmode`` for the port configuration. > - > * cryptodev: support for using IV with all sizes is added, J0 still can > be used but only when IV length in following structs ``rte_crypto_auth= _xform``, > ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or= equal > diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_note= s/release_19_11.rst > index 87b7bd0..a3fc023 100644 > --- a/doc/guides/rel_notes/release_19_11.rst > +++ b/doc/guides/rel_notes/release_19_11.rst > @@ -418,6 +418,14 @@ ABI Changes > align the Ethernet header on receive and all known encapsulations > preserve the alignment of the header. >=20 > +* ethdev: Added 32-bit fields for maximum LRO aggregated packet size, in > + struct ``rte_eth_dev_info`` for the port capability and in struct > + ``rte_eth_rxmode`` for the port configuration. > + Application should use the new field in struct ``rte_eth_rxmode`` to c= onfigure > + the requested size. That part I am not happy with: * application should use*. Many apps I suppose are ok with default LRO size selected by PMD/HW. Why to force changes in all of them? > + PMD should use the new field in struct ``rte_eth_dev_info`` to report = the > + supported port capability. > + >=20 > Shared Library Versions > ----------------------- > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethde= v.c > index b9b055e..741b897 100644 > --- a/drivers/net/bnxt/bnxt_ethdev.c > +++ b/drivers/net/bnxt/bnxt_ethdev.c > @@ -519,6 +519,7 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev *e= th_dev, > /* Fast path specifics */ > dev_info->min_rx_bufsize =3D 1; > dev_info->max_rx_pktlen =3D BNXT_MAX_PKT_LEN; > + dev_info->max_lro_pkt_size =3D BNXT_MAX_PKT_LEN; >=20 > dev_info->rx_offload_capa =3D BNXT_DEV_RX_OFFLOAD_SUPPORT; > if (bp->flags & BNXT_FLAG_PTP_SUPPORTED) > diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hin= ic_pmd_ethdev.c > index 9f37a40..b33b2cf 100644 > --- a/drivers/net/hinic/hinic_pmd_ethdev.c > +++ b/drivers/net/hinic/hinic_pmd_ethdev.c > @@ -727,6 +727,7 @@ static void hinic_get_speed_capa(struct rte_eth_dev *= dev, uint32_t *speed_capa) > info->max_tx_queues =3D nic_dev->nic_cap.max_sqs; > info->min_rx_bufsize =3D HINIC_MIN_RX_BUF_SIZE; > info->max_rx_pktlen =3D HINIC_MAX_JUMBO_FRAME_SIZE; > + info->max_lro_pkt_size =3D HINIC_MAX_JUMBO_FRAME_SIZE; > info->max_mac_addrs =3D HINIC_MAX_UC_MAC_ADDRS; > info->min_mtu =3D HINIC_MIN_MTU_SIZE; > info->max_mtu =3D HINIC_MAX_MTU_SIZE; > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_e= thdev.c > index 30c0379..5719552 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -3814,6 +3814,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unuse= d struct rte_eth_dev *dev, > } > dev_info->min_rx_bufsize =3D 1024; /* cf BSIZEPACKET in SRRCTL register= */ > dev_info->max_rx_pktlen =3D 15872; /* includes CRC, cf MAXFRS register = */ > + dev_info->max_lro_pkt_size =3D RTE_IPV4_MAX_PKT_LEN; > dev_info->max_mac_addrs =3D hw->mac.num_rar_entries; > dev_info->max_hash_mac_addrs =3D IXGBE_VMDQ_NUM_UC_MAC; > dev_info->max_vfs =3D pci_dev->max_vfs; > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index fab58c9..4783b5c 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -206,6 +206,9 @@ struct mlx5_hca_attr { > #define MLX5_LRO_SUPPORTED(dev) \ > (((struct mlx5_priv *)((dev)->data->dev_private))->config.lro.supported= ) >=20 > +/* Maximal size of aggregated LRO packet. */ > +#define MLX5_MAX_LRO_SIZE (UINT8_MAX * 256u) > + > /* LRO configurations structure. */ > struct mlx5_lro_config { > uint32_t supported:1; /* Whether LRO is supported. */ > diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethde= v.c > index 2b7c867..3adc824 100644 > --- a/drivers/net/mlx5/mlx5_ethdev.c > +++ b/drivers/net/mlx5/mlx5_ethdev.c > @@ -606,6 +606,7 @@ struct ethtool_link_settings { > /* FIXME: we should ask the device for these values. */ > info->min_rx_bufsize =3D 32; > info->max_rx_pktlen =3D 65536; > + info->max_lro_pkt_size =3D MLX5_MAX_LRO_SIZE; > /* > * Since we need one CQ per QP, the limit is the minimum number > * between the two values. > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c > index 24d0eaa..9423e7b 100644 > --- a/drivers/net/mlx5/mlx5_rxq.c > +++ b/drivers/net/mlx5/mlx5_rxq.c > @@ -1701,7 +1701,6 @@ struct mlx5_rxq_obj * > return 0; > } >=20 > -#define MLX5_MAX_LRO_SIZE (UINT8_MAX * 256u) > #define MLX5_MAX_TCP_HDR_OFFSET ((unsigned int)(sizeof(struct rte_ether_= hdr) + \ > sizeof(struct rte_vlan_hdr) * 2 + \ > sizeof(struct rte_ipv6_hdr))) > diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethde= v.c > index 575982f..ccbb8a4 100644 > --- a/drivers/net/qede/qede_ethdev.c > +++ b/drivers/net/qede/qede_ethdev.c > @@ -1277,6 +1277,7 @@ static int qede_dev_configure(struct rte_eth_dev *e= th_dev) >=20 > dev_info->min_rx_bufsize =3D (uint32_t)QEDE_MIN_RX_BUFF_SIZE; > dev_info->max_rx_pktlen =3D (uint32_t)ETH_TX_MAX_NON_LSO_PKT_LEN; > + dev_info->max_lro_pkt_size =3D (uint32_t)0x7FFF; > dev_info->rx_desc_lim =3D qede_rx_desc_lim; > dev_info->tx_desc_lim =3D qede_tx_desc_lim; >=20 > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virt= io_ethdev.c > index 044eb10..22ce5a2 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -2435,6 +2435,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_de= v *dev) > RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_TX_QUEUES); > dev_info->min_rx_bufsize =3D VIRTIO_MIN_RX_BUFSIZE; > dev_info->max_rx_pktlen =3D VIRTIO_MAX_RX_PKTLEN; > + dev_info->max_lro_pkt_size =3D VIRTIO_MAX_RX_PKTLEN; > dev_info->max_mac_addrs =3D VIRTIO_MAX_MAC_ADDRS; >=20 > host_features =3D VTPCI_OPS(hw)->get_features(hw); > diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/v= mxnet3_ethdev.c > index d1faeaa..d18e8bc 100644 > --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c > +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c > @@ -1161,6 +1161,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_de= vice *pci_dev) > dev_info->max_tx_queues =3D VMXNET3_MAX_TX_QUEUES; > dev_info->min_rx_bufsize =3D 1518 + RTE_PKTMBUF_HEADROOM; > dev_info->max_rx_pktlen =3D 16384; /* includes CRC, cf MAXFRS register = */ > + dev_info->max_lro_pkt_size =3D 16384; > dev_info->speed_capa =3D ETH_LINK_SPEED_10G; > dev_info->max_mac_addrs =3D VMXNET3_MAX_MAC_ADDRS; >=20 > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethde= v.c > index 652c369..c642ba5 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1136,6 +1136,26 @@ struct rte_eth_dev * > return name; > } >=20 > +static inline int > +check_lro_pkt_size(uint16_t port_id, uint32_t config_size, > + uint32_t dev_info_size) > +{ > + int ret =3D 0; > + > + if (config_size > dev_info_size) { > + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=3D%d max_lro_pkt_size %u " > + "> max allowed value %u\n", port_id, config_size, > + dev_info_size); > + ret =3D -EINVAL; > + } else if (config_size < RTE_ETHER_MIN_LEN) { > + RTE_ETHDEV_LOG(ERR, "Ethdev port_id=3D%d max_lro_pkt_size %u " > + "< min allowed value %u\n", port_id, config_size, > + (unsigned int)RTE_ETHER_MIN_LEN); > + ret =3D -EINVAL; > + } > + return ret; > +} > + > int > rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx= _q, > const struct rte_eth_conf *dev_conf) > @@ -1266,6 +1286,18 @@ struct rte_eth_dev * > RTE_ETHER_MAX_LEN; > } >=20 > + /* > + * If LRO is enabled, check that the maximum aggregated packet > + * size is supported by the configured device. > + */ > + if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) { > + ret =3D check_lro_pkt_size( > + port_id, dev_conf->rxmode.max_lro_pkt_size, > + dev_info.max_lro_pkt_size); > + if (ret !=3D 0) > + goto rollback; > + } > + > /* Any requested offloading must be within its device capabilities */ > if ((dev_conf->rxmode.offloads & dev_info.rx_offload_capa) !=3D > dev_conf->rxmode.offloads) { > @@ -1770,6 +1802,18 @@ struct rte_eth_dev * > return -EINVAL; > } >=20 > + /* > + * If LRO is enabled, check that the maximum aggregated packet > + * size is supported by the configured device. > + */ > + if (local_conf.offloads & DEV_RX_OFFLOAD_TCP_LRO) { > + int ret =3D check_lro_pkt_size(port_id, > + dev->data->dev_conf.rxmode.max_lro_pkt_size, > + dev_info.max_lro_pkt_size); > + if (ret !=3D 0) > + return ret; > + } > + > ret =3D (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc, > socket_id, &local_conf, mp); > if (!ret) { > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethde= v.h > index 44d77b3..1b76df5 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -395,6 +395,8 @@ struct rte_eth_rxmode { > /** The multi-queue packet distribution mode to be used, e.g. RSS. */ > enum rte_eth_rx_mq_mode mq_mode; > uint32_t max_rx_pkt_len; /**< Only used if JUMBO_FRAME enabled. */ > + /** Maximum allowed size of LRO aggregated packet. */ > + uint32_t max_lro_pkt_size; > uint16_t split_hdr_size; /**< hdr buf size (header_split enabled).*/ > /** > * Per-port Rx offloads to be set using DEV_RX_OFFLOAD_* flags. > @@ -1218,6 +1220,8 @@ struct rte_eth_dev_info { > const uint32_t *dev_flags; /**< Device flags */ > uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */ > uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */ > + /** Maximum configurable size of LRO aggregated packet. */ > + uint32_t max_lro_pkt_size; > uint16_t max_rx_queues; /**< Maximum number of RX queues. */ > uint16_t max_tx_queues; /**< Maximum number of TX queues. */ > uint32_t max_mac_addrs; /**< Maximum number of MAC addresses. */ > -- > 1.8.3.1