diff mbox series

[v3,3/3] net/af_xdp: fix maximum MTU value

Message ID 20200210114009.49590-4-ciara.loftus@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series AF_XDP PMD Fixes | expand

Checks

Context Check Description
ci/Intel-compilation fail apply issues
ci/travis-robot success Travis build: passed
ci/checkpatch success coding style OK

Commit Message

Ciara Loftus Feb. 10, 2020, 11:40 a.m. UTC 
The maximum MTU for af_xdp zero copy is equal to the page size less the
frame overhead introduced by AF_XDP (XDP HR = 256) and DPDK (frame headroom
= 320). The patch updates this value to reflect this.

This change also makes it possible to remove unneeded constants for both
zero-copy and copy mode.

Fixes: d8a210774e1d ("net/af_xdp: support unaligned umem chunks")
Cc: stable@dpdk.org

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Comments

Xiaolong Ye Feb. 13, 2020, 3:05 a.m. UTC | #1 
On 02/10, Ciara Loftus wrote:
>The maximum MTU for af_xdp zero copy is equal to the page size less the
>frame overhead introduced by AF_XDP (XDP HR = 256) and DPDK (frame headroom
>= 320). The patch updates this value to reflect this.
>
>This change also makes it possible to remove unneeded constants for both
>zero-copy and copy mode.
>
>Fixes: d8a210774e1d ("net/af_xdp: support unaligned umem chunks")
>Cc: stable@dpdk.org
>
>Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
>---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
>index 1e98cd44f..75f037c3e 100644
>--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>@@ -59,13 +59,6 @@ static int af_xdp_logtype;
> 
> #define ETH_AF_XDP_FRAME_SIZE		2048
> #define ETH_AF_XDP_NUM_BUFFERS		4096
>-#ifdef XDP_UMEM_UNALIGNED_CHUNK_FLAG
>-#define ETH_AF_XDP_MBUF_OVERHEAD	128 /* sizeof(struct rte_mbuf) */
>-#define ETH_AF_XDP_DATA_HEADROOM \
>-	(ETH_AF_XDP_MBUF_OVERHEAD + RTE_PKTMBUF_HEADROOM)
>-#else
>-#define ETH_AF_XDP_DATA_HEADROOM	0
>-#endif
> #define ETH_AF_XDP_DFLT_NUM_DESCS	XSK_RING_CONS__DEFAULT_NUM_DESCS
> #define ETH_AF_XDP_DFLT_START_QUEUE_IDX	0
> #define ETH_AF_XDP_DFLT_QUEUE_COUNT	1
>@@ -602,7 +595,14 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> 	dev_info->max_tx_queues = internals->queue_cnt;
> 
> 	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
>-	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM;
>+#if defined(XDP_UMEM_UNALIGNED_CHUNK_FLAG)
>+	dev_info->max_mtu = getpagesize() -
>+				sizeof(struct rte_mempool_objhdr) -
>+				sizeof(struct rte_mbuf) -
>+				RTE_PKTMBUF_HEADROOM - XDP_PACKET_HEADROOM;
>+#else
>+	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE;

Do we need to subtract XDP_PACKET_HEADROOM for copy mode as well? 

Thanks,
Xiaolong

>+#endif
> 
> 	dev_info->default_rxportconf.nb_queues = 1;
> 	dev_info->default_txportconf.nb_queues = 1;
>@@ -804,7 +804,7 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
> 		.fill_size = ETH_AF_XDP_DFLT_NUM_DESCS,
> 		.comp_size = ETH_AF_XDP_DFLT_NUM_DESCS,
> 		.frame_size = ETH_AF_XDP_FRAME_SIZE,
>-		.frame_headroom = ETH_AF_XDP_DATA_HEADROOM };
>+		.frame_headroom = 0 };
> 	char ring_name[RTE_RING_NAMESIZE];
> 	char mz_name[RTE_MEMZONE_NAMESIZE];
> 	int ret;
>@@ -829,8 +829,7 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
> 
> 	for (i = 0; i < ETH_AF_XDP_NUM_BUFFERS; i++)
> 		rte_ring_enqueue(umem->buf_ring,
>-				 (void *)(i * ETH_AF_XDP_FRAME_SIZE +
>-					  ETH_AF_XDP_DATA_HEADROOM));
>+				 (void *)(i * ETH_AF_XDP_FRAME_SIZE));
> 
> 	snprintf(mz_name, sizeof(mz_name), "af_xdp_umem_%s_%u",
> 		       internals->if_name, rxq->xsk_queue_idx);
>@@ -939,7 +938,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
> 	/* Now get the space available for data in the mbuf */
> 	buf_size = rte_pktmbuf_data_room_size(mb_pool) -
> 		RTE_PKTMBUF_HEADROOM;
>-	data_size = ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM;
>+	data_size = ETH_AF_XDP_FRAME_SIZE;
> 
> 	if (data_size > buf_size) {
> 		AF_XDP_LOG(ERR, "%s: %d bytes will not fit in mbuf (%d bytes)\n",
>-- 
>2.17.1
>
Ciara Loftus Feb. 13, 2020, 8:43 a.m. UTC | #2 
> Subject: Re: [dpdk-dev] [PATCH v3 3/3] net/af_xdp: fix maximum MTU value
> 
> On 02/10, Ciara Loftus wrote:
> >The maximum MTU for af_xdp zero copy is equal to the page size less the
> >frame overhead introduced by AF_XDP (XDP HR = 256) and DPDK (frame
> headroom
> >= 320). The patch updates this value to reflect this.
> >
> >This change also makes it possible to remove unneeded constants for both
> >zero-copy and copy mode.
> >
> >Fixes: d8a210774e1d ("net/af_xdp: support unaligned umem chunks")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> >---
> > drivers/net/af_xdp/rte_eth_af_xdp.c | 23 +++++++++++------------
> > 1 file changed, 11 insertions(+), 12 deletions(-)
> >
> >diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> >index 1e98cd44f..75f037c3e 100644
> >--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> >+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> >@@ -59,13 +59,6 @@ static int af_xdp_logtype;
> >
> > #define ETH_AF_XDP_FRAME_SIZE		2048
> > #define ETH_AF_XDP_NUM_BUFFERS		4096
> >-#ifdef XDP_UMEM_UNALIGNED_CHUNK_FLAG
> >-#define ETH_AF_XDP_MBUF_OVERHEAD	128 /* sizeof(struct
> rte_mbuf) */
> >-#define ETH_AF_XDP_DATA_HEADROOM \
> >-	(ETH_AF_XDP_MBUF_OVERHEAD + RTE_PKTMBUF_HEADROOM)
> >-#else
> >-#define ETH_AF_XDP_DATA_HEADROOM	0
> >-#endif
> > #define ETH_AF_XDP_DFLT_NUM_DESCS
> 	XSK_RING_CONS__DEFAULT_NUM_DESCS
> > #define ETH_AF_XDP_DFLT_START_QUEUE_IDX	0
> > #define ETH_AF_XDP_DFLT_QUEUE_COUNT	1
> >@@ -602,7 +595,14 @@ eth_dev_info(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> > 	dev_info->max_tx_queues = internals->queue_cnt;
> >
> > 	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
> >-	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE -
> ETH_AF_XDP_DATA_HEADROOM;
> >+#if defined(XDP_UMEM_UNALIGNED_CHUNK_FLAG)
> >+	dev_info->max_mtu = getpagesize() -
> >+				sizeof(struct rte_mempool_objhdr) -
> >+				sizeof(struct rte_mbuf) -
> >+				RTE_PKTMBUF_HEADROOM -
> XDP_PACKET_HEADROOM;
> >+#else
> >+	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE;
> 
> Do we need to subtract XDP_PACKET_HEADROOM for copy mode as well?

Good catch. I'll add this and spin a v4.
Thanks for the reviews.

Ciara

> 
> Thanks,
> Xiaolong
> 
> >+#endif
> >
> > 	dev_info->default_rxportconf.nb_queues = 1;
> > 	dev_info->default_txportconf.nb_queues = 1;
> >@@ -804,7 +804,7 @@ xsk_umem_info *xdp_umem_configure(struct
> pmd_internals *internals,
> > 		.fill_size = ETH_AF_XDP_DFLT_NUM_DESCS,
> > 		.comp_size = ETH_AF_XDP_DFLT_NUM_DESCS,
> > 		.frame_size = ETH_AF_XDP_FRAME_SIZE,
> >-		.frame_headroom = ETH_AF_XDP_DATA_HEADROOM };
> >+		.frame_headroom = 0 };
> > 	char ring_name[RTE_RING_NAMESIZE];
> > 	char mz_name[RTE_MEMZONE_NAMESIZE];
> > 	int ret;
> >@@ -829,8 +829,7 @@ xsk_umem_info *xdp_umem_configure(struct
> pmd_internals *internals,
> >
> > 	for (i = 0; i < ETH_AF_XDP_NUM_BUFFERS; i++)
> > 		rte_ring_enqueue(umem->buf_ring,
> >-				 (void *)(i * ETH_AF_XDP_FRAME_SIZE +
> >-					  ETH_AF_XDP_DATA_HEADROOM));
> >+				 (void *)(i * ETH_AF_XDP_FRAME_SIZE));
> >
> > 	snprintf(mz_name, sizeof(mz_name), "af_xdp_umem_%s_%u",
> > 		       internals->if_name, rxq->xsk_queue_idx);
> >@@ -939,7 +938,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
> > 	/* Now get the space available for data in the mbuf */
> > 	buf_size = rte_pktmbuf_data_room_size(mb_pool) -
> > 		RTE_PKTMBUF_HEADROOM;
> >-	data_size = ETH_AF_XDP_FRAME_SIZE -
> ETH_AF_XDP_DATA_HEADROOM;
> >+	data_size = ETH_AF_XDP_FRAME_SIZE;
> >
> > 	if (data_size > buf_size) {
> > 		AF_XDP_LOG(ERR, "%s: %d bytes will not fit in mbuf (%d
> bytes)\n",
> >--
> >2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 1e98cd44f..75f037c3e 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -59,13 +59,6 @@  static int af_xdp_logtype;
 
 #define ETH_AF_XDP_FRAME_SIZE		2048
 #define ETH_AF_XDP_NUM_BUFFERS		4096
-#ifdef XDP_UMEM_UNALIGNED_CHUNK_FLAG
-#define ETH_AF_XDP_MBUF_OVERHEAD	128 /* sizeof(struct rte_mbuf) */
-#define ETH_AF_XDP_DATA_HEADROOM \
-	(ETH_AF_XDP_MBUF_OVERHEAD + RTE_PKTMBUF_HEADROOM)
-#else
-#define ETH_AF_XDP_DATA_HEADROOM	0
-#endif
 #define ETH_AF_XDP_DFLT_NUM_DESCS	XSK_RING_CONS__DEFAULT_NUM_DESCS
 #define ETH_AF_XDP_DFLT_START_QUEUE_IDX	0
 #define ETH_AF_XDP_DFLT_QUEUE_COUNT	1
@@ -602,7 +595,14 @@  eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->queue_cnt;
 
 	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
-	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM;
+#if defined(XDP_UMEM_UNALIGNED_CHUNK_FLAG)
+	dev_info->max_mtu = getpagesize() -
+				sizeof(struct rte_mempool_objhdr) -
+				sizeof(struct rte_mbuf) -
+				RTE_PKTMBUF_HEADROOM - XDP_PACKET_HEADROOM;
+#else
+	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE;
+#endif
 
 	dev_info->default_rxportconf.nb_queues = 1;
 	dev_info->default_txportconf.nb_queues = 1;
@@ -804,7 +804,7 @@  xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
 		.fill_size = ETH_AF_XDP_DFLT_NUM_DESCS,
 		.comp_size = ETH_AF_XDP_DFLT_NUM_DESCS,
 		.frame_size = ETH_AF_XDP_FRAME_SIZE,
-		.frame_headroom = ETH_AF_XDP_DATA_HEADROOM };
+		.frame_headroom = 0 };
 	char ring_name[RTE_RING_NAMESIZE];
 	char mz_name[RTE_MEMZONE_NAMESIZE];
 	int ret;
@@ -829,8 +829,7 @@  xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
 
 	for (i = 0; i < ETH_AF_XDP_NUM_BUFFERS; i++)
 		rte_ring_enqueue(umem->buf_ring,
-				 (void *)(i * ETH_AF_XDP_FRAME_SIZE +
-					  ETH_AF_XDP_DATA_HEADROOM));
+				 (void *)(i * ETH_AF_XDP_FRAME_SIZE));
 
 	snprintf(mz_name, sizeof(mz_name), "af_xdp_umem_%s_%u",
 		       internals->if_name, rxq->xsk_queue_idx);
@@ -939,7 +938,7 @@  eth_rx_queue_setup(struct rte_eth_dev *dev,
 	/* Now get the space available for data in the mbuf */
 	buf_size = rte_pktmbuf_data_room_size(mb_pool) -
 		RTE_PKTMBUF_HEADROOM;
-	data_size = ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM;
+	data_size = ETH_AF_XDP_FRAME_SIZE;
 
 	if (data_size > buf_size) {
 		AF_XDP_LOG(ERR, "%s: %d bytes will not fit in mbuf (%d bytes)\n",