diff mbox

[dpdk-dev] net/bnx2x: reserve enough headroom for mbuf prepend

Message ID 6fd6d52e-a756-45ad-9666-60fa7ca676cd@zyc-PC.local (mailing list archive)
State Rejected, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Matt Feb. 6, 2018, 11:21 a.m. UTC 
  Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
---
 drivers/net/bnx2x/bnx2x_rxtx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Ferruh Yigit Feb. 6, 2018, 1:44 p.m. UTC | #1 
On 2/6/2018 11:21 AM, zhouyangchao wrote:
> Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>

Hi Yangchao,

There are multiple version of this patch and all seems marked as rejected in
patchwork, intentionally?

If not can you please update the correct one as new in patchwork?
Ferruh Yigit March 5, 2018, 3:28 p.m. UTC | #2 
On 2/6/2018 11:21 AM, zhouyangchao wrote:

Can you please provide more information why this patch is needed?

> Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
> ---
>  drivers/net/bnx2x/bnx2x_rxtx.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c
> index a0d4ac9..d8a3225 100644
> --- a/drivers/net/bnx2x/bnx2x_rxtx.c
> +++ b/drivers/net/bnx2x/bnx2x_rxtx.c
> @@ -140,7 +140,8 @@ bnx2x_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  			return -ENOMEM;
>  		}
>  		rxq->sw_ring[idx] = mbuf;
> -		rxq->rx_ring[idx] = mbuf->buf_iova;
> +		rxq->rx_ring[idx] = 
> +			rte_cpu_to_le_64(rte_mbuf_data_iova_default(mbuf));
>  	}
>  	rxq->pkt_first_seg = NULL;
>  	rxq->pkt_last_seg = NULL;
> @@ -400,7 +401,8 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  
>  		rx_mb = rxq->sw_ring[bd_cons];
>  		rxq->sw_ring[bd_cons] = new_mb;
> -		rxq->rx_ring[bd_prod] = new_mb->buf_iova;
> +		rxq->rx_ring[bd_prod] = 
> +			rte_cpu_to_le_64(rte_mbuf_data_iova_default(new_mb));
>  
>  		rx_pref = NEXT_RX_BD(bd_cons) & MAX_RX_BD(rxq);
>  		rte_prefetch0(rxq->sw_ring[rx_pref]);
> @@ -409,7 +411,7 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  			rte_prefetch0(&rxq->sw_ring[rx_pref]);
>  		}
>  
> -		rx_mb->data_off = pad;
> +		rx_mb->data_off = pad + RTE_PKTMBUF_HEADROOM;
>  		rx_mb->nb_segs = 1;
>  		rx_mb->next = NULL;
>  		rx_mb->pkt_len = rx_mb->data_len = len;
>
Matt March 8, 2018, 5:57 a.m. UTC | #3 
When allocating a new mbuf for Rx, the value of m->data_off should be
reset to its default value (RTE_PKTMBUF_HEADROOM), instead of reusing
the previous undefined value, which could cause the packet to have a too
small or too high headroom.

On Mon, Mar 5, 2018 at 11:28 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 2/6/2018 11:21 AM, zhouyangchao wrote:
>
> Can you please provide more information why this patch is needed?
>
> > Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
> > ---
> >  drivers/net/bnx2x/bnx2x_rxtx.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c
> b/drivers/net/bnx2x/bnx2x_rxtx.c
> > index a0d4ac9..d8a3225 100644
> > --- a/drivers/net/bnx2x/bnx2x_rxtx.c
> > +++ b/drivers/net/bnx2x/bnx2x_rxtx.c
> > @@ -140,7 +140,8 @@ bnx2x_dev_rx_queue_setup(struct rte_eth_dev *dev,
> >                       return -ENOMEM;
> >               }
> >               rxq->sw_ring[idx] = mbuf;
> > -             rxq->rx_ring[idx] = mbuf->buf_iova;
> > +             rxq->rx_ring[idx] =
> > +                     rte_cpu_to_le_64(rte_mbuf_data_iova_default(mbuf));
> >       }
> >       rxq->pkt_first_seg = NULL;
> >       rxq->pkt_last_seg = NULL;
> > @@ -400,7 +401,8 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
> >
> >               rx_mb = rxq->sw_ring[bd_cons];
> >               rxq->sw_ring[bd_cons] = new_mb;
> > -             rxq->rx_ring[bd_prod] = new_mb->buf_iova;
> > +             rxq->rx_ring[bd_prod] =
> > +
>  rte_cpu_to_le_64(rte_mbuf_data_iova_default(new_mb));
> >
> >               rx_pref = NEXT_RX_BD(bd_cons) & MAX_RX_BD(rxq);
> >               rte_prefetch0(rxq->sw_ring[rx_pref]);
> > @@ -409,7 +411,7 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
> >                       rte_prefetch0(&rxq->sw_ring[rx_pref]);
> >               }
> >
> > -             rx_mb->data_off = pad;
> > +             rx_mb->data_off = pad + RTE_PKTMBUF_HEADROOM;
> >               rx_mb->nb_segs = 1;
> >               rx_mb->next = NULL;
> >               rx_mb->pkt_len = rx_mb->data_len = len;
> >
>
>
Ferruh Yigit April 20, 2018, 10:31 a.m. UTC | #4 
On 3/8/2018 5:57 AM, zhouyangchao wrote:
> When allocating a new mbuf for Rx, the value of m->data_off should be 
> reset to its default value (RTE_PKTMBUF_HEADROOM), instead of reusing 
> the previous undefined value, which could cause the packet to have a too 
> small or too high headroom.

Hi Harish, Rasesh,

Reminder of this patch waiting for your review?

> 
> On Mon, Mar 5, 2018 at 11:28 PM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 2/6/2018 11:21 AM, zhouyangchao wrote:
> 
>     Can you please provide more information why this patch is needed?
> 
>     > Signed-off-by: Yangchao Zhou <zhouyates@gmail.com
>     <mailto:zhouyates@gmail.com>>
>     > ---
>     >  drivers/net/bnx2x/bnx2x_rxtx.c | 8 +++++---
>     >  1 file changed, 5 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c
>     > index a0d4ac9..d8a3225 100644
>     > --- a/drivers/net/bnx2x/bnx2x_rxtx.c
>     > +++ b/drivers/net/bnx2x/bnx2x_rxtx.c
>     > @@ -140,7 +140,8 @@ bnx2x_dev_rx_queue_setup(struct rte_eth_dev *dev,
>     >                       return -ENOMEM;
>     >               }
>     >               rxq->sw_ring[idx] = mbuf;
>     > -             rxq->rx_ring[idx] = mbuf->buf_iova;
>     > +             rxq->rx_ring[idx] =
>     > +                     rte_cpu_to_le_64(rte_mbuf_data_iova_default(mbuf));
>     >       }
>     >       rxq->pkt_first_seg = NULL;
>     >       rxq->pkt_last_seg = NULL;
>     > @@ -400,7 +401,8 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf
>     **rx_pkts, uint16_t nb_pkts)
>     >
>     >               rx_mb = rxq->sw_ring[bd_cons];
>     >               rxq->sw_ring[bd_cons] = new_mb;
>     > -             rxq->rx_ring[bd_prod] = new_mb->buf_iova;
>     > +             rxq->rx_ring[bd_prod] =
>     > +                     rte_cpu_to_le_64(rte_mbuf_data_iova_default(new_mb));
>     >
>     >               rx_pref = NEXT_RX_BD(bd_cons) & MAX_RX_BD(rxq);
>     >               rte_prefetch0(rxq->sw_ring[rx_pref]);
>     > @@ -409,7 +411,7 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf
>     **rx_pkts, uint16_t nb_pkts)
>     >                       rte_prefetch0(&rxq->sw_ring[rx_pref]);
>     >               }
>     >
>     > -             rx_mb->data_off = pad;
>     > +             rx_mb->data_off = pad + RTE_PKTMBUF_HEADROOM;
>     >               rx_mb->nb_segs = 1;
>     >               rx_mb->next = NULL;
>     >               rx_mb->pkt_len = rx_mb->data_len = len;
>     >
>
Patil, Harish April 23, 2018, 5:10 p.m. UTC | #5 
From: zhouyangchao <zhouyates@gmail.com<mailto:zhouyates@gmail.com>>

Date: Wednesday, March 7, 2018 at 10:57 PM
To: Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>>
Cc: "dev@dpdk.org<mailto:dev@dpdk.org>" <dev@dpdk.org<mailto:dev@dpdk.org>>, Harish Patil <Harish.Patil@cavium.com<mailto:Harish.Patil@cavium.com>>, "Mody, Rasesh" <Rasesh.Mody@cavium.com<mailto:Rasesh.Mody@cavium.com>>
Subject: Re: [dpdk-dev] [PATCH] net/bnx2x: reserve enough headroom for mbuf prepend

When allocating a new mbuf for Rx, the value of m->data_off should be
reset to its default value (RTE_PKTMBUF_HEADROOM), instead of reusing
the previous undefined value, which could cause the packet to have a too
small or too high headroom.

On Mon, Mar 5, 2018 at 11:28 PM Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>> wrote:
On 2/6/2018 11:21 AM, zhouyangchao wrote:

Can you please provide more information why this patch is needed?

> Signed-off-by: Yangchao Zhou <zhouyates@gmail.com<mailto:zhouyates@gmail.com>>

> ---

>  drivers/net/bnx2x/bnx2x_rxtx.c | 8 +++++---

>  1 file changed, 5 insertions(+), 3 deletions(-)

>

> diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c

> index a0d4ac9..d8a3225 100644

> --- a/drivers/net/bnx2x/bnx2x_rxtx.c

> +++ b/drivers/net/bnx2x/bnx2x_rxtx.c

> @@ -140,7 +140,8 @@ bnx2x_dev_rx_queue_setup(struct rte_eth_dev *dev,

>                       return -ENOMEM;

>               }

>               rxq->sw_ring[idx] = mbuf;

> -             rxq->rx_ring[idx] = mbuf->buf_iova;

> +             rxq->rx_ring[idx] =

> +                     rte_cpu_to_le_64(rte_mbuf_data_iova_default(mbuf));

>       }

>       rxq->pkt_first_seg = NULL;

>       rxq->pkt_last_seg = NULL;

> @@ -400,7 +401,8 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)

>

>               rx_mb = rxq->sw_ring[bd_cons];

>               rxq->sw_ring[bd_cons] = new_mb;

> -             rxq->rx_ring[bd_prod] = new_mb->buf_iova;

> +             rxq->rx_ring[bd_prod] =

> +                     rte_cpu_to_le_64(rte_mbuf_data_iova_default(new_mb));

>

>               rx_pref = NEXT_RX_BD(bd_cons) & MAX_RX_BD(rxq);

>               rte_prefetch0(rxq->sw_ring[rx_pref]);

> @@ -409,7 +411,7 @@ bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)

>                       rte_prefetch0(&rxq->sw_ring[rx_pref]);

>               }

>

> -             rx_mb->data_off = pad;

> +             rx_mb->data_off = pad + RTE_PKTMBUF_HEADROOM;

>               rx_mb->nb_segs = 1;

>               rx_mb->next = NULL;

>               rx_mb->pkt_len = rx_mb->data_len = len;

>



Acked-by: Harish Patil <harish.patil@cavium.com<mailto:harish.patil@cavium.com>>
Ferruh Yigit April 24, 2018, 12:38 p.m. UTC | #6 
On 4/23/2018 6:10 PM, Patil, Harish wrote:
> From: zhouyangchao <zhouyates@gmail.com <mailto:zhouyates@gmail.com>>
> Date: Wednesday, March 7, 2018 at 10:57 PM
> To: Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>>
> Cc: "dev@dpdk.org <mailto:dev@dpdk.org>" <dev@dpdk.org <mailto:dev@dpdk.org>>,
> Harish Patil <Harish.Patil@cavium.com <mailto:Harish.Patil@cavium.com>>, "Mody,
> Rasesh" <Rasesh.Mody@cavium.com <mailto:Rasesh.Mody@cavium.com>>
> Subject: Re: [dpdk-dev] [PATCH] net/bnx2x: reserve enough headroom for mbuf prepend
> 
>     When allocating a new mbuf for Rx, the value of m->data_off should be 
>     reset to its default value (RTE_PKTMBUF_HEADROOM), instead of reusing 
>     the previous undefined value, which could cause the packet to have a too 
>     small or too high headroom.
> 
>     On Mon, Mar 5, 2018 at 11:28 PM Ferruh Yigit <ferruh.yigit@intel.com
>     <mailto:ferruh.yigit@intel.com>> wrote:
> 
>         On 2/6/2018 11:21 AM, zhouyangchao wrote:
> 
>         Can you please provide more information why this patch is needed?
> 
>         > Signed-off-by: Yangchao Zhou <zhouyates@gmail.com <mailto:zhouyates@gmail.com>>

<...>

> Acked-by: Harish Patil <harish.patil@cavium.com <mailto:harish.patil@cavium.com>>

Applied to dpdk-next-net/master, thanks.
diff mbox

Patch

diff --git a/drivers/net/bnx2x/bnx2x_rxtx.c b/drivers/net/bnx2x/bnx2x_rxtx.c
index a0d4ac9..d8a3225 100644
--- a/drivers/net/bnx2x/bnx2x_rxtx.c
+++ b/drivers/net/bnx2x/bnx2x_rxtx.c
@@ -140,7 +140,8 @@  bnx2x_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			return -ENOMEM;
 		}
 		rxq->sw_ring[idx] = mbuf;
-		rxq->rx_ring[idx] = mbuf->buf_iova;
+		rxq->rx_ring[idx] = 
+			rte_cpu_to_le_64(rte_mbuf_data_iova_default(mbuf));
 	}
 	rxq->pkt_first_seg = NULL;
 	rxq->pkt_last_seg = NULL;
@@ -400,7 +401,8 @@  bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 		rx_mb = rxq->sw_ring[bd_cons];
 		rxq->sw_ring[bd_cons] = new_mb;
-		rxq->rx_ring[bd_prod] = new_mb->buf_iova;
+		rxq->rx_ring[bd_prod] = 
+			rte_cpu_to_le_64(rte_mbuf_data_iova_default(new_mb));
 
 		rx_pref = NEXT_RX_BD(bd_cons) & MAX_RX_BD(rxq);
 		rte_prefetch0(rxq->sw_ring[rx_pref]);
@@ -409,7 +411,7 @@  bnx2x_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			rte_prefetch0(&rxq->sw_ring[rx_pref]);
 		}
 
-		rx_mb->data_off = pad;
+		rx_mb->data_off = pad + RTE_PKTMBUF_HEADROOM;
 		rx_mb->nb_segs = 1;
 		rx_mb->next = NULL;
 		rx_mb->pkt_len = rx_mb->data_len = len;