[dpdk-dev] example/l2fwd-crypto: maybe an issue?
Hi, Do we have assumption each core will have a separate crypto device? I have only one crypto device with several queues which are shared by multi-cores. As I run the l2fwd-crypto, only one cparam is valid and has a good dev_id. The other cparam is crashing. The reason is the other qconf assoicated with the core is not initializing the dev_id since only one crypto device is available. Thanks, Forrest
[dpdk-dev] [PATCH v4] enforce rules of the cpu and ixgbe exchange data.
Hi Thomas & Konstantin, Thanks for the review and the comments are addressed by http://www.dpdk.org/dev/patchwork/patch/6653/ Best Regards, Xuelin Shi > -Origina Konstantin l Message- > From: Ananyev, Konstantin [mailto:konstantin.ananyev at intel.com] > Sent: Monday, July 27, 2015 22:43 > To: Thomas Monjalon > Cc: Shi Xuelin-B29237; dev at dpdk.org > Subject: RE: [PATCH v4] enforce rules of the cpu and ixgbe exchange data. > > > > > -Original Message- > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > Sent: Monday, July 27, 2015 3:18 PM > > To: Ananyev, Konstantin > > Cc: xuelin.shi at freescale.com; dev at dpdk.org > > Subject: Re: [PATCH v4] enforce rules of the cpu and ixgbe exchange > data. > > > > A quick review of this long pending patch would be great. > > Thanks > > Well, it doesn't compile: > > /local/kananye1/dpdk.org-ixgbevfix2-tst1/drivers/net/ixgbe/ixgbe_rxtx.c: > In function ?ixgbe_rx_scan_hw_ring?: > /local/kananye1/dpdk.org-ixgbevfix2- > tst1/drivers/net/ixgbe/ixgbe_rxtx.c:1114:4: error: implicit declaration > of function ?rte_le_to_cpu16? [-Werror=implicit-function-declaration] > pkt_len = rte_le_to_cpu16(rxdp[j].wb.upper.length) - > ^ > /local/kananye1/dpdk.org-ixgbevfix2- > tst1/drivers/net/ixgbe/ixgbe_rxtx.c:1114:4: error: nested extern > declaration of ?rte_le_to_cpu16? [-Werror=nested-externs] > > > Should be rte_le_to_cpu_16(), I believe. > > And checkpatch.pl complains on it: > > WARNING: line over 80 characters > #151: FILE: drivers/net/ixgbe/ixgbe_rxtx.c:1133: > + > + rte_le_to_cpu_32(rxdp[j].wb.lower.lo_dword.data)); > > ERROR: code indent should use tabs where possible > #170: FILE: drivers/net/ixgbe/ixgbe_rxtx.c:1147: > +^I^I^I ^Irxdp[j].wb.lower.hi_dword.csum_ip.csum) &$ > > WARNING: please, no space before tabs > #170: FILE: drivers/net/ixgbe/ixgbe_rxtx.c:1147: > +^I^I^I ^Irxdp[j].wb.lower.hi_dword.csum_ip.csum) &$ > > total: 1 errors, 2 warnings, 192 lines checked > > Apart from that, looks harmless :) > > Konstantin > > > > > 2015-07-16 14:45, xuelin.shi at freescale.com: > > > From: Xuelin Shi > > > > > > 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu > > > fill data to ixgbe must use rte_cpu_to_le_xx(...) 3. checking pci > > > status with converted constant. > > > > > > Signed-off-by: Xuelin Shi
[dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu
Hi Thomas,
I have worked out the new version based on dpdk v2 codebase to address the
comments.
http://www.dpdk.org/dev/patchwork/patch/6449/
thanks,
Xuelin Shi
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, July 10, 2015 22:52
> To: Shi Xuelin-B29237
> Cc: Ananyev, Konstantin; dev at dpdk.org
> Subject: Re: [PATCH v3] ixgbe: fix data access on big endian cpu
>
> Xuelin, have you given up with that patch?
>
> 2015-04-14 23:11, Ananyev, Konstantin:
> > From: xuelin.shi at freescale.com [mailto:xuelin.shi at freescale.com]
> > > txd->read.olinfo_status =
> > > - rte_cpu_to_le_32(olinfo_status);
> > > + rte_cpu_to_le_32(olinfo_status);
> > > +
> >
> > Not sure, what had changed here?
>
> > > @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev
> *dev, uint16_t rx_queue_id)
> > > rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> > >
> > > while ((desc < rxq->nb_rx_desc) &&
> > > - (rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD)) {
> > > + (rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> > > + IXGBE_RXDADV_STAT_DD)) {
> >
> > Why not ' rxdp->wb.upper.status_error &
> rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)'?
> > To keep it consistent with rest of the changes?
>
>
[dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu
Hi,
OK, I'm fine to defer it.
Thanks,
Xuelin Shi
> -Original Message-
> From: Butler, Siobhan A [mailto:siobhan.a.butler at intel.com]
> Sent: Friday, April 03, 2015 04:18
> To: Shi Xuelin-B29237; Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian
> cpu
>
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of
> > xuelin.shi at freescale.com
> > Sent: Tuesday, March 31, 2015 8:26 AM
> > To: Ananyev, Konstantin
> > Cc: dev at dpdk.org; Xuelin Shi
> > Subject: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian
> > cpu
> >
> > From: Xuelin Shi
> >
> > enforce rules of the cpu and ixgbe exchange data.
> > 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu
> > fill data to ixgbe must use rte_cpu_to_le_xx(...) 3. checking pci
> > status with converted constant.
> >
> > Signed-off-by: Xuelin Shi
> > ---
> > change for v3:
> >check pci status with converted constant to avoid performance
> penalty.
> >remove tmp variable.
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 89
> > ---
> >
> > 1 file changed, 56 insertions(+), 33 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index 9da2c7e..6e508ec 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -129,7 +129,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
> >
> > /* check DD bit on threshold descriptor */
> > status = txq->tx_ring[txq->tx_next_dd].wb.status;
> > - if (! (status & IXGBE_ADVTXD_STAT_DD))
> > + if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
> > return 0;
> >
> > /*
> > @@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp,
> > struct rte_mbuf **pkts)
> > pkt_len = (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr = buf_dma_addr;
> > + txdp->read.buffer_addr =
> > rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> > txdp->read.olinfo_status =
> > - (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > + rte_cpu_to_le_32(pkt_len <<
> > IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> > rte_prefetch0(&(*pkts)->pool);
> > }
> > }
> > @@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp,
> > struct rte_mbuf **pkts)
> > pkt_len = (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr = buf_dma_addr;
> > + txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> > txdp->read.olinfo_status =
> > - (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > + rte_cpu_to_le_32(pkt_len <<
> > IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> > rte_prefetch0(&(*pkts)->pool);
> > }
> >
> > @@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts,
> > * a divisor of the ring size
> > */
> > tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> >
> > txq->tx_tail = 0;
> > @@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts,
> > */
> > if (txq->tx_tail > txq->tx_next_rs) {
> > tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
> > txq->tx_rs_thresh);
> > if
[dpdk-dev] [PATCH v2] ixgbe: fix ixgbe access endian issue on bigendian arch
Hi,
Please see my comments inline.
Thanks,
Xuelin
> -Original Message-
> From: Ananyev, Konstantin [mailto:konstantin.ananyev at intel.com]
> Sent: Friday, March 27, 2015 09:04
> To: Shi Xuelin-B29237; thomas.monjalon at 6wind.com
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] ixgbe: fix ixgbe access endian issue
> on bigendian arch
>
> Hi,
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of
> > xuelin.shi at freescale.com
> > Sent: Tuesday, March 24, 2015 6:34 AM
> > To: thomas.monjalon at 6wind.com
> > Cc: dev at dpdk.org; Xuelin Shi
> > Subject: [dpdk-dev] [PATCH v2] ixgbe: fix ixgbe access endian issue on
> > bigendian arch
> >
> > From: Xuelin Shi
> >
> > enforce rules of the cpu and ixgbe exchange data.
> > 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu
> > fill data to ixgbe must use rte_cpu_to_le_xx(...)
> >
> > Signed-off-by: Xuelin Shi
> > ---
> > changes for v2:
> > rebased on latest head.
> > fix some style issue detected by checkpatch.pl
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 104
> > --
> > 1 file changed, 66 insertions(+), 38 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index 9da2c7e..961ac08 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -128,7 +128,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
> > int i;
> >
> > /* check DD bit on threshold descriptor */
> > - status = txq->tx_ring[txq->tx_next_dd].wb.status;
> > + status = rte_le_to_cpu_32(txq->tx_ring[txq->tx_next_dd].wb.status);
> > if (! (status & IXGBE_ADVTXD_STAT_DD))
> > return 0;
> >
> > @@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp,
> struct rte_mbuf **pkts)
> > pkt_len = (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr = buf_dma_addr;
> > + txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > +
> > txdp->read.olinfo_status =
> > - (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > + rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> > rte_prefetch0(&(*pkts)->pool);
> > }
> > }
> > @@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp,
> struct rte_mbuf **pkts)
> > pkt_len = (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr = buf_dma_addr;
> > + txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > +
> > txdp->read.olinfo_status =
> > - (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > + rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> > rte_prefetch0(&(*pkts)->pool);
> > }
> >
> > @@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
> > * a divisor of the ring size
> > */
> > tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > + rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> >
> > txq->tx_tail = 0;
> > @@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
> > */
> > if (txq->tx_tail > txq->tx_next_rs) {
> > tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > + rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
> > txq->tx_rs_thresh);
> > if (txq->tx_next_rs >= txq->nb_tx_desc) @@ -505,6 +511,7 @@
&
[dpdk-dev] [PATCH] ixgbe: fix data access on big endian cpu
Hi Thomas,
Done. http://patchwork.dpdk.org/dev/patchwork/patch/4123/
Thanks,
Xuelin Shi
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, March 23, 2015 22:02
> To: Shi Xuelin-B29237
> Cc: dev at dpdk.org; konstantin.ananyev at intel.com; helin.zhang at intel.com
> Subject: Re: [PATCH] ixgbe: fix data access on big endian cpu
>
> 2015-03-03 16:27, xuelin.shi at freescale.com:
> > From: Xuelin Shi
> >
> > enforce rules for cpu and ixgbe exchanging data.
> > 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu
> > fill data to ixgbe must use rte_cpu_to_le_xx(...)
> >
> > Signed-off-by: Xuelin Shi
>
> Please Xuelin, could you rebase on HEAD and fix these checkpatch errors?
>
> ERROR:SPACING: space prohibited after that '!' (ctx:BxW)
>
> ERROR:CODE_INDENT: code indent should use tabs where possible
> +^I^I ^I ^I IXGBE_RXDADV_STAT_DD)) {$
>
> Thanks
[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian
Hi Thomas, Done. http://patchwork.dpdk.org/dev/patchwork/patch/4122/ Thanks, Xuelin Shi > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, March 23, 2015 22:04 > To: Shi Xuelin-B29237 > Cc: Bruce Richardson; dev at dpdk.org > Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big > endian > > 2015-03-09 14:02, Bruce Richardson: > > On Wed, Mar 04, 2015 at 02:34:12PM +0800, xuelin.shi at freescale.com > wrote: > > > From: Xuelin Shi > > > > > > This module uses type conversion between struct and int. > > > Also truncation and comparison is used with this int. > > > It is not safe for different endian arch. > > > > > > Add ifdef for big endian struct to fix this issue. > > > > > > Signed-off-by: Xuelin Shi > > > > Get an error compiling this up (using clang on FreeBSD). > > > > CC rte_lpm.o > > In file included from > /usr/home/bruce/dpdk.org/lib/librte_lpm/rte_lpm.c:57: > > /usr/home/bruce/dpdk.org/lib/librte_lpm/rte_lpm.h:99:5: fatal error: > > 'RTE_BYTE_ORDER' is not defined, evaluates to 0 [-Wundef] #if > RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN > > ^ > > 1 error generated. > > > > Adding "#include " should fix the issue. > > Please Xuelin, could you submit a v2? > Thanks > > > Existing unit tests on IA (little endian) pass fine there-after, but I > > think for this patch it would be good to have an ack from someone who > > can validate on a big endian system, since this is what this patch is > meant to enable. > > > > /Bruce > > >
[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian
Hi, OK, done. Marked as "not applicable". Thanks, Shi > -Original Message- > From: Mcnamara, John [mailto:john.mcnamara at intel.com] > Sent: Monday, March 09, 2015 17:13 > To: Shi Xuelin-B29237; Richardson, Bruce > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely > for big endian > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xuelin Shi > > Sent: Monday, March 9, 2015 1:54 AM > > To: Richardson, Bruce > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely > > for big endian > > > > Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be > > abandoned. > > > > Hi, > > If you register and login to the DPDK patchwork site you can mark the > patch as "Not Applicable" or whatever category now applies. > > http://dpdk.org/dev/patchwork/user/login/ > > John
[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian
Hi Bruce,
Yes, the patch http://dpdk.org/dev/patchwork/patch/3132/ should be abandoned.
Thanks,
Shi xuelin
> -Original Message-
> From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> Sent: Friday, March 06, 2015 19:14
> To: Shi Xuelin-B29237
> Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big
> endian
>
> On Thu, Mar 05, 2015 at 02:12:12AM +, Xuelin Shi wrote:
> > Hi Bruce,
> >
> > Yes, it needs to swap the fields. The bit field is first identified as
> the uint8_t and then packed.
> >
> > Thanks,
> > Shi xuelin
> >
> Am I right in thinking that this patch set supercedes that for
> "lpm: use field access instead of type conversion"
> http://dpdk.org/dev/patchwork/patch/3132/ ?
>
> > > -Original Message-
> > > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> > > Sent: Wednesday, March 04, 2015 18:48
> > > To: Shi Xuelin-B29237
> > > Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> > > Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big
> > > endian
> > >
> > > On Wed, Mar 04, 2015 at 02:34:12PM +0800, xuelin.shi at freescale.com
> wrote:
> > > > From: Xuelin Shi
> > > >
> > > > This module uses type conversion between struct and int.
> > > > Also truncation and comparison is used with this int.
> > > > It is not safe for different endian arch.
> > > >
> > > > Add ifdef for big endian struct to fix this issue.
> > > >
> > > > Signed-off-by: Xuelin Shi
> > > > ---
> > > > lib/librte_lpm/rte_lpm.h | 19 +++
> > > > 1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
> > > > index
> > > > 1af150c..08a2859 100644
> > > > --- a/lib/librte_lpm/rte_lpm.h
> > > > +++ b/lib/librte_lpm/rte_lpm.h
> > > > @@ -96,6 +96,7 @@ extern "C" {
> > > > /** Bitmask used to indicate successful lookup */
> > > > #define RTE_LPM_LOOKUP_SUCCESS 0x0100
> > > >
> > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > /** @internal Tbl24 entry structure. */ struct
> rte_lpm_tbl24_entry {
> > > > /* Stores Next hop or group index (i.e. gindex)into tbl8. */
> @@
> > > > -117,6 +118,24 @@ struct rte_lpm_tbl8_entry {
> > > > uint8_t valid_group :1; /**< Group validation flag. */
> > > > uint8_t depth :6; /**< Rule depth. */
> > > > };
> > > > +#else
> > > > +struct rte_lpm_tbl24_entry {
> > > > + uint8_t depth :6;
> > > > + uint8_t ext_entry :1;
> > > > + uint8_t valid :1;
> > >
> > > Since endianness only refers to the order of bytes within a word, do
> > > the bitfields within the uint8_t really need to be swapped around too?
> > >
> > > /Bruce
> >
> > >
> > > > + union {
> > > > + uint8_t tbl8_gindex;
> > > > + uint8_t next_hop;
> > > > + };
> > > > +};
> > > > +
> > > > +struct rte_lpm_tbl8_entry {
> > > > + uint8_t depth :6;
> > > > + uint8_t valid_group :1;
> > > > + uint8_t valid :1;
> > > > + uint8_t next_hop;
> > > > +};
> > > > +#endif
> > > >
> > > > /** @internal Rule structure. */
> > > > struct rte_lpm_rule {
> > > > --
> > > > 1.9.1
> > > >
[dpdk-dev] [PATCH] librte_lpm: define tbl entry reversely for big endian
Hi Bruce,
Yes, it needs to swap the fields. The bit field is first identified as the
uint8_t and then packed.
Thanks,
Shi xuelin
> -Original Message-
> From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> Sent: Wednesday, March 04, 2015 18:48
> To: Shi Xuelin-B29237
> Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> Subject: Re: [PATCH] librte_lpm: define tbl entry reversely for big
> endian
>
> On Wed, Mar 04, 2015 at 02:34:12PM +0800, xuelin.shi at freescale.com wrote:
> > From: Xuelin Shi
> >
> > This module uses type conversion between struct and int.
> > Also truncation and comparison is used with this int.
> > It is not safe for different endian arch.
> >
> > Add ifdef for big endian struct to fix this issue.
> >
> > Signed-off-by: Xuelin Shi
> > ---
> > lib/librte_lpm/rte_lpm.h | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index
> > 1af150c..08a2859 100644
> > --- a/lib/librte_lpm/rte_lpm.h
> > +++ b/lib/librte_lpm/rte_lpm.h
> > @@ -96,6 +96,7 @@ extern "C" {
> > /** Bitmask used to indicate successful lookup */
> > #define RTE_LPM_LOOKUP_SUCCESS 0x0100
> >
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > /** @internal Tbl24 entry structure. */ struct rte_lpm_tbl24_entry {
> > /* Stores Next hop or group index (i.e. gindex)into tbl8. */ @@
> > -117,6 +118,24 @@ struct rte_lpm_tbl8_entry {
> > uint8_t valid_group :1; /**< Group validation flag. */
> > uint8_t depth :6; /**< Rule depth. */
> > };
> > +#else
> > +struct rte_lpm_tbl24_entry {
> > + uint8_t depth :6;
> > + uint8_t ext_entry :1;
> > + uint8_t valid :1;
>
> Since endianness only refers to the order of bytes within a word, do the
> bitfields within the uint8_t really need to be swapped around too?
>
> /Bruce
>
> > + union {
> > + uint8_t tbl8_gindex;
> > + uint8_t next_hop;
> > + };
> > +};
> > +
> > +struct rte_lpm_tbl8_entry {
> > + uint8_t depth :6;
> > + uint8_t valid_group :1;
> > + uint8_t valid :1;
> > + uint8_t next_hop;
> > +};
> > +#endif
> >
> > /** @internal Rule structure. */
> > struct rte_lpm_rule {
> > --
> > 1.9.1
> >
[dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion.
Hi,
Needs more consideration.
RTE_LPM_VALID_EXT_ENTRY_BITMASK is defined as 0x0030, also little endian
assumed.
Seems like the struct bit field also need some position conversion.
I would like to send v2 patch to fix that.
Thanks,
Shi Xuelin
> -Original Message-
> From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> Sent: Thursday, February 12, 2015 19:18
> To: Shi Xuelin-B29237
> Cc: thomas.monjalon at 6wind.com; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] librte_lpm: use field access instead of
> type conversion.
>
> On Wed, Feb 11, 2015 at 02:12:59PM +0800, xuelin.shi at freescale.com wrote:
> > From: Xuelin Shi
> >
> > struct tbl_entry{
> > uint8_t next_hop;
> > uint8_t valid :1;
> > uint8_t valid_group :1;
> > uint8_t depth :6
> > }
> > uint16_t tbl = (uint16_t)tbl_entry;
> > next_hop = (uint8_t)tbl;
> >
> > next_hop cannot get the correct value of the field if the cpu arch is
> > BIG_ENDIAN.
> >
> > change it to field access.
> >
> > Signed-off-by: Xuelin Shi
> > ---
> > lib/librte_lpm/rte_lpm.h | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index
> > 586300b..1af150c 100644
> > --- a/lib/librte_lpm/rte_lpm.h
> > +++ b/lib/librte_lpm/rte_lpm.h
> > @@ -273,6 +273,7 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip,
> > uint8_t *next_hop) {
> > unsigned tbl24_index = (ip >> 8);
> > uint16_t tbl_entry;
> > + struct rte_lpm_tbl8_entry *entry;
> >
> > /* DEBUG: Check user input arguments. */
> > RTE_LPM_RETURN_IF_TRUE(((lpm == NULL) || (next_hop == NULL)),
> > -EINVAL); @@ -290,8 +291,10 @@ rte_lpm_lookup(struct rte_lpm *lpm,
> uint32_t ip, uint8_t *next_hop)
> > tbl_entry = *(const uint16_t *)>tbl8[tbl8_index];
> > }
> >
> > - *next_hop = (uint8_t)tbl_entry;
> > - return (tbl_entry & RTE_LPM_LOOKUP_SUCCESS) ? 0 : -ENOENT;
> > + entry = (struct rte_lpm_tbl8_entry *)_entry;
> > + *next_hop = entry->next_hop;
> > +
> > + return (entry->valid) ? 0 : -ENOENT;
> > }
> >
> > /**
> > --
> > 1.9.1
> >
> I've run a quick test using "lpm_autotest" inside the test app, and on my
> (Intel) platform, this patch has a small (but none-the-less significant)
> performance regression. How about the below as an alternative fix?
>
> /Bruce
>
> diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index
> 586300b..de6f1cb 100644
> --- a/lib/librte_lpm/rte_lpm.h
> +++ b/lib/librte_lpm/rte_lpm.h
> @@ -44,6 +44,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -287,7 +288,8 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip,
> uint8_t *next_hop)
> unsigned tbl8_index = (uint8_t)ip +
> ((uint8_t)tbl_entry *
> RTE_LPM_TBL8_GROUP_NUM_ENTRIES);
>
> - tbl_entry = *(const uint16_t *)>tbl8[tbl8_index];
> + tbl_entry = rte_cpu_to_le_16(
> + *(const uint16_t
> + *)>tbl8[tbl8_index]);
> }
>
> *next_hop = (uint8_t)tbl_entry;
[dpdk-dev] [PATCH] fix testpmd show port info error
Hi Olivier,
Would you please help to review the bottom patch?
Thanks,
Shi Xuelin
-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Xuelin Shi
Sent: Thursday, February 05, 2015 17:27
To: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH] fix testpmd show port info error
Hi,
Anybody interested in this patch could have a review or comment on it?
I'm new here. Should I send this patch to some specific maintainer to make this
more efficient?
Thanks,
Shi Xuelin
-Original Message-
From: xuelin.shi at freescale.com [mailto:xuelin@freescale.com]
Sent: Monday, February 02, 2015 14:52
To: dev at dpdk.org
Cc: Shi Xuelin-B29237
Subject: [PATCH] fix testpmd show port info error
From: Xuelin Shi <xuelin@freescale.com>
the port number type should be consistent with librte_cmdline, else there is
potential endian issue.
Signed-off-by: Xuelin Shi
---
app/test-pmd/cmdline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
4beb404..488ac63 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5568,7 +5568,7 @@ cmdline_parse_token_string_t cmd_showport_what =
TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
"info#stats#xstats#fdir#stat_qmap");
cmdline_parse_token_num_t cmd_showport_portnum =
- TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, INT32);
+ TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, INT8);
cmdline_parse_inst_t cmd_showport = {
.f = cmd_showport_parsed,
--
1.9.1
[dpdk-dev] [PATCH] fix testpmd show port info error
Hi,
Anybody interested in this patch could have a review or comment on it?
I'm new here. Should I send this patch to some specific maintainer to make this
more efficient?
Thanks,
Shi Xuelin
-Original Message-
From: xuelin.shi at freescale.com [mailto:xuelin@freescale.com]
Sent: Monday, February 02, 2015 14:52
To: dev at dpdk.org
Cc: Shi Xuelin-B29237
Subject: [PATCH] fix testpmd show port info error
From: Xuelin Shi <xuelin@freescale.com>
the port number type should be consistent with librte_cmdline, else there is
potential endian issue.
Signed-off-by: Xuelin Shi
---
app/test-pmd/cmdline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
4beb404..488ac63 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5568,7 +5568,7 @@ cmdline_parse_token_string_t cmd_showport_what =
TOKEN_STRING_INITIALIZER(struct cmd_showport_result, what,
"info#stats#xstats#fdir#stat_qmap");
cmdline_parse_token_num_t cmd_showport_portnum =
- TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, INT32);
+ TOKEN_NUM_INITIALIZER(struct cmd_showport_result, portnum, INT8);
cmdline_parse_inst_t cmd_showport = {
.f = cmd_showport_parsed,
--
1.9.1
